Skip to content

Conversation

@RubenVerborgh
Copy link
Contributor

@RubenVerborghRubenVerborgh commented Aug 17, 2017

Implements #246 with WAC-Allow: user="read write append",public="read".

@csarven
Copy link
Member

Good stuff!

Would "WAC-Mode" be preferable just to keep it closer to *nix's mode bits?

@RubenVerborgh
Copy link
ContributorAuthor

@csarven The header name was chosen in a discussion with @timbl and @dmitrizagidulin yesterday (see #246). I don't mind the naming either way (although I still think "WAC" is cryptic).

@dmitrizagidulin
Copy link
Contributor

I just realized we want to do WAC-Allow, minus the ed on the end. (To match the HTTP Allow: header). Sorry bout that :)

@RubenVerborghRubenVerborghforce-pushed the feature/permissions-in-header branch 3 times, most recently from e7e7911 to b792b9aCompareAugust 17, 2017 21:37
Copy link
Contributor

@timbltimbl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the whiteboard we had spaces or commas to separate the modes, to mimic other headers. In this code we have semicolons... was that a conscious decision?

@RubenVerborgh
Copy link
ContributorAuthor

Which of these would you prefer? I don't mind either way; I was following what @dmitrizagidulin wrote down of our discussion in #246.

@csarven
Copy link
Member

To stick close to convention (like in Web Linking for relation types), probably best to use space as a delimiter. Plus, it is easier to read.

@RubenVerborgh
Copy link
ContributorAuthor

Makes sense! Are the quotes needed?

@csarven
Copy link
Member

I think the quotes might make it easier for consumers to tokenise. It can also help to escape characters or have specialised modes or something in the future.

@RubenVerborgh
Copy link
ContributorAuthor

Okay, mandatory quotes it is.

@RubenVerborghRubenVerborghforce-pushed the feature/permissions-in-header branch from b792b9a to 250e513CompareAugust 18, 2017 13:28
@RubenVerborgh
Copy link
ContributorAuthor

250e513 brings spaces.

@RubenVerborgh
Copy link
ContributorAuthor

Currently on hold because of #552.

@RubenVerborghRubenVerborghforce-pushed the feature/permissions-in-header branch from 250e513 to 4b6a381CompareAugust 18, 2017 18:58
@RubenVerborgh
Copy link
ContributorAuthor

#552 has been merged, so this one is ready for review and merge.

Copy link
Contributor

@dmitrizagidulindmitrizagidulin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been a much-requested feature! Nicely done!

@dmitrizagidulindmitrizagidulin merged commit 2b8f18b into dz_oidcAug 18, 2017
@dmitrizagidulindmitrizagidulin deleted the feature/permissions-in-header branch August 18, 2017 19:09
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@RubenVerborgh@csarven@dmitrizagidulin@timbl