Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add auth.md to document provider authentication #81

Merged

Conversation

asadowns
Copy link
Contributor

Provide information on authentication with JWT and minimal payload.

Update provider/README.md to reference auth.md

Refs Issue #60

Provide information on authentication with JWT and minimal payload.

Update provider/README.md to reference auth.md

Refs Issue [openmobilityfoundation#60](openmobilityfoundation#60)
@asadowns
Copy link
Contributor Author

@hunterowens let me know if this is what you were looking for. Happy to revise. Wasn't entirely sure how much detail to provide in auth.md.

@migurski
Copy link

I’m not convinced that JWT should be a part of the MDS spec. From the standpoint of a mobility provider, JWT might be a great solution to the problem of token generation. From the perspective of an API consumer, it doesn’t seem relevant how a particular bearer token was generated as long as it can be used to gain access to a resource.

@noonhub
Copy link
Contributor

noonhub commented Sep 20, 2018

I somewhat agree with @migurski here. However, if we do not standardize on the format and how to retrieve the token, then I think the provider should be allowed to enforce any bearer token based authentication scheme.

Is the current proposal assuming that the provider generates this JWT and issues it to the agency manually? What is the TLL of the tokens?

If we do want to standardize on the format and interface to generate these tokens, OAuth 2.0 client_credentials grant type as laid out in RFC 7523 would be a good candidate for that standard.

I am in favor of leaving it up to the provider, but this means that the agency may need to implement some flow to retrieve a bearer token for each provider.

@thekaveman
Copy link
Collaborator

Regardless of the particular implementation / token format, I think it's useful to have a single standard baked into MDS. Agencies aren't going to want to implement N different flows for N different providers.

And on the flip side, when the agency API takes off, we'll hopefully have a corresponding flow for Providers which is straightforward for any/all Agencies they interact with.

@migurski
Copy link

Not sure I agree @thekaveman. Even if JWT is chosen as a format it doesn’t specify the agency/provider token flow at all. One way to implement a token flow is via OAuth which places the onus on providers to develop a 3rd party developer key infrastructure. Even Github offers personal access tokens as an external-to-OAuth ad hoc way to get tokens.

Specifying Authorization header bearer tokens in MDS makes sense, but the means by which those tokens are generated and shared seems like scope overreach at this stage.

@asadowns
Copy link
Contributor Author

@migurski and @noonhub I think your guys points are valid. I agree with @noonhub that ultimately as long as a bearer token is provided it doesn't make a huge difference what the format is.

All that said, JWT does seem like a good standard to cement around for provider and agency API. And specifying an auth method does have the benefit of providing clear instructions for providers as to what they should provide which helps avoid confusion.

We haven't discussed any sort of refresh/login flow or endpoints so right now the TTL on the tokens we would provide to the city is one year. In order not to add the extra burden of a login/refresh flow to the agencies it does seem worthwhile to set a long TTL.

I don't necessary think it's overreach to provide info on an authorization method. But, I think it's very reasonable to make additional PRs to add more authorization methods. Thoughts, @thekaveman ?

@thekaveman
Copy link
Collaborator

Specifying Authorization header bearer tokens in MDS makes sense, but the means by which those tokens are generated and shared seems like scope overreach at this stage

I think this is what I tried (meant? wanted?) to say, but you said it much clearer @migurski 😄

I guess I mixed concepts - what I'm concerned about (as an agency that will be consuming provider data from 4 providers to start) is having a single type of auth method. I don't want to send a username/password to one, Authorization: <token> to another, SMS 2-Factor for another, blood sample for yet another... So any kind of clarification around that aspect is what I am looking for, and I think you said it correctly above.

For the use-cases currently outlined (straight auth, plus tying geographic areas to specific agencies) I think JWT is compelling. But it does seem like more of a provider implementation detail than something specifically needed in the spec (and ditto for TTL) -- at least at this point, still so early in the game.

@migurski
Copy link

++ @thekaveman, that’s great. Bearer tokens are a clean, simple way to do this. JWT could be offered as a suggestion on how to generate good ones.

@asadowns
Copy link
Contributor Author

Sounds like we're all in agreement on bearer tokens. Please feel free to merge / close / or request revisions to PR based on what you think belongs in the spec at this point. Thanks!

provider/auth.md Outdated
@@ -0,0 +1,11 @@
# Authorization

Authorization for **provider** API should be provided via a JSON Web Token or [JWT](https://jwt.io/introduction/).
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps pivot this document away from discussing JWTs and towards specifying that authorization for the API will be communicated using Bearer Tokens, specified in the Authorization header, as detailed in IETF RFC 6750. Then a section can discuss how the spec does not dictate how those tokens are generated and transmitted to consumers, suggesting JWT as a common approach?

Copy link
Collaborator

Choose a reason for hiding this comment

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

^ @oderby that would be great.

noonhub
noonhub previously approved these changes Sep 25, 2018
Copy link
Contributor

@noonhub noonhub left a comment

Choose a reason for hiding this comment

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

These changes look good to me.

We may also want to recommend that these tokens are generated using the OAuth specification on the provider's platform to make life easier on the agency, but that does not seem like a blocker to merging this change.

making Authorization header example more explicit
Copy link
Collaborator

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

I had a couple minor fixes, but LGTM

@hunterowens hunterowens changed the base branch from master to dev September 26, 2018 03:51
@hunterowens hunterowens merged commit d7c5570 into openmobilityfoundation:dev Sep 26, 2018
hunterowens pushed a commit that referenced this pull request Oct 1, 2018
* Add auth.md to document provider authentication. Fixes #60 

Provide information on authentication with JWT and minimal payload.

Update provider/README.md to reference auth.md

Refs Issue [#60](#60)

* Update Auth to do bearer token screen.

* cleaning up formatting

making Authorization header example more explicit

* fix link to auth document
hunterowens added a commit that referenced this pull request Oct 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants