Merged
Conversation
mirw
reviewed
Sep 22, 2017
| - Start of changelog. | ||
|
|
||
| [Unreleased]: https://github.com/olivierlacan/keep-a-changelog/compare/0.5.0...HEAD | ||
| [0.5.0]: https://github.com/olivierlacan/keep-a-changelog/compare/0.4.0...0.5.0 |
| pub scopes: Scopes, | ||
| /// Authenticated identity of the party to which | ||
| /// authorization has been granted, if available | ||
| /// (i.e., who is doing the accessing). |
Contributor
There was a problem hiding this comment.
Is "who is doing the accessing" actually correct here (or, at least, is it over simplifying)? If I make a request to microservice X to access microservice Y to retrieve data from store Z, you could say that X, Y and Z are all doing the accessing, but I think the issuer would still be me. Correct? Maybe it's better to either drop the simplified explanation, or flesh it out?
Contributor
Author
There was a problem hiding this comment.
I've fleshed it out a bit, with reference to RFC6749 and RFC7519. Hopefully this is better?
Contributor
|
Looks good. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After discussion re #14 we agreed to add
issuer- knowing who did something is very important for audit purposes; just knowing who it was done to is not enough. Also it is useful to be able to report per-issuer (i.e., per-third-party-app) statistics from server applications, and this is easier if that information is present within the application (rather than just in logs).We agreed to drop
auth_typeandexpiry_deadline. The first (auth_type) would encourage services to have auth-type-dependent behaviour, making the testing matrix more complex; the second (expiry_deadline) is not actually useful - authenticators may wish to do internal caching based on deadlines, but the validity period of the auth data doesn't imply anything about the validity period of the requester's authorization.This is a breaking change (new field in struct), hence there is a version bump. If approved, I'll tag and release the library to cargo.