Skip to content

Implement authorization_code AuthenticationProvider #68

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

Closed
jgrandja opened this issue Apr 24, 2020 · 15 comments
Closed

Implement authorization_code AuthenticationProvider #68

jgrandja opened this issue Apr 24, 2020 · 15 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@jgrandja
Copy link
Collaborator

jgrandja commented Apr 24, 2020

The OAuth2AuthorizationCodeAuthenticationProvider is responsible for authenticating the authorization code parameter.

Implementation Requirements

  • the OAuth2TokenEndpointFilter Implement Token Endpoint #67 indirectly calls this AuthenticationProvider by passing in OAuth2AuthorizationCodeAuthenticationToken
  • the RegisteredClientRepository Implement Client Registration Model / Repository #40 should be used to validate the client_id parameter if the client was not previously authenticated
  • the OAuth2AuthorizationService Implement Authorization Model / Service #43 should be used to lookup the OAuth2Authorization using the code parameter
  • the accessTokenGenerator should be used to generate an opaque access token. NOTE: This will later be re-factored to generate a JWT
  • the access token should be returned by OAuth2AuthorizationCodeAuthenticationProvider in a OAuth2AccessTokenAuthenticationToken
  • javadoc class and public methods
  • Unit tests

Specification References

3.1. Token Endpoint
4.1. Authorization Code Grant
4.1.3. Access Token Request
4.1.4. Access Token Response

@jgrandja jgrandja added type: enhancement A general enhancement status: on-hold We can't start working on this issue yet and removed type: enhancement A general enhancement labels Apr 24, 2020
@jgrandja jgrandja added this to the 0.0.1 milestone Apr 24, 2020
@jgrandja jgrandja added status: ideal-for-contribution An issue that we actively are looking for someone to help us with type: enhancement A general enhancement and removed status: on-hold We can't start working on this issue yet labels Apr 24, 2020
@dfcoffin
Copy link

@jgrandja We had discussed the ability to allow an implementation to enhance the generated access token response, but I don't see that described above in the Implementation Requirements

@dfcoffin
Copy link

dfcoffin commented Apr 26, 2020

@jgrandja Is it possible to update the Implementation Requirements section with Issue numbers?

@jgrandja
Copy link
Collaborator Author

jgrandja commented Apr 27, 2020

@dfcoffin I updated the issues to cross reference the dependent issues.

Customizing the token response will come in a later iteration. The implementation requirements are for this iteration only. We will likely need to go through 3 or more iterations to fully realize the authorization_code grant, which will include full customizations as per the framework.

@dfcoffin
Copy link

@jgrandja Thanks for the update. After reading several of your PR review comments, I'm getting a better feeling for the overall project plan and code development approach. Is this issue still available?

@jgrandja
Copy link
Collaborator Author

@dfcoffin This issue is available. Would you like to take it?

@dfcoffin
Copy link

@jgrandja Yes.

@jgrandja
Copy link
Collaborator Author

Thanks @dfcoffin. The issue is yours.

@jgrandja jgrandja removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Apr 27, 2020
@dfcoffin
Copy link

@jgrandja I'm a bit confused as to which contributor is working on this issue. I reviewed the numerous comments submitted in the last 24 hours. It seems other contributors have implemented some of the outlined packages and methods (i.e., Authentication Provider) as components of other assigned issues.

What are your suggestions so I am not duplicating code already developed by other contributors, although not part of their assigned issues? Perhaps when a step covers multiple issues, all relevant issues need to be referenced in the issue description? Is there a means contributors can better collaborate than posting comments?

@jgrandja
Copy link
Collaborator Author

@dfcoffin

I'm a bit confused as to which contributor is working on this issue

When an issue is assigned, we assign the generic spring-contributor. The general process is that a user should ask to work on the issue first, and if available indicated by status: ideal-for-contribution, then we will respond with the "issue is yours". Then we assign spring-contributor and remove status: ideal-for-contribution. This issue is assigned to you.

It seems other contributors have implemented some of the outlined packages and methods (i.e., Authentication Provider)

Which issue are you referring to? The AuthenticationProvider that needs to be delivered in this issue is OAuth2AuthorizationCodeAuthenticationProvider.

@jgrandja
Copy link
Collaborator Author

@dfcoffin I updated a couple of the points in the main issue description. Hopefully that clarifies things?

This issue should implement the following:

OAuth2AuthorizationCodeAuthenticationToken (input) -> OAuth2AuthorizationCodeAuthenticationProvider -> OAuth2AccessTokenAuthenticationToken (output)

@paurav-munshi
Copy link
Contributor

@jgrandja @dfcoffin

Looking at this issue and #67 there is some confusion in my mind. This issue deals with creating a AuthenticationProvider implementation which will take OAuth2AuthorizationCodeAuthenticationToken as input. This seems in place for this issue as this Provider will validate the code, redirect_uri and client_id. But what confuses me is why would this provider generate a token. Actually OAuth2TokenEndpointFilter in #67 should generate the token using accessTokenGenerator . It would be the responsibility of the filter. Right ? The provider is only supposed to perform the authentication. So in my opinion provider should a) take input OAuth2AuthorizationCodeAuthenticationToken b) perform validation using RegisteredClientRepository and OAuth2AuthorizationService c) set isAuthenticated or other necessary attributes d) return same object. And then OAuth2TokenEndpointFilter in #67 should generate the token after checking the isAuthenticated of OAuth2AuthorizationCodeAuthenticationToken.

Additionally, the class OAuth2AccessTokenAuthenticationToken should be used in token introspection where token validation will be performed. What is the use of creating this Authentication implementation when we are validating authorization_code and not token.

So far from what I have seen, the common pattern is that Filter orchestrates between various components which perform single operation like coverter, matcher, generator etc to achieve a desired business function. This class performs two things which seems a bit odd to me.

@jgrandja
Copy link
Collaborator Author

@dfcoffin How is the PR coming along? Do you have any questions or need help with anything?

@dfcoffin
Copy link

@jgrandja Unfortunately due to other work-related projects I haven't had a chance to make much progress. If I am holding up moving to the next set of objectives, I'm happy to let someone with more time tackle this issue.

@paurav-munshi
Copy link
Contributor

@jgrandja @dfcoffin
If it gets unassigned then I would be interested in working on this issue. Let me know if I can contribute.

Thanks,
Paurav.

@jgrandja
Copy link
Collaborator Author

No worries @dfcoffin.

Thanks for the offer @paurav-munshi. Let's get your current PR merged first and then we can look at other tasks. I will likely take this one on as I'm striving to get the first iteration done for authorization code grant sometime next week.

@anoopgarlapati anoopgarlapati mentioned this issue Jun 11, 2020
9 tasks
doba16 pushed a commit to doba16/spring-authorization-server that referenced this issue Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants