Implement Authorization Endpoint

[jgrandja]

An authorization server provides an authorization endpoint, which it uses when interacting with the resource owner to obtain authorization for a client.

The OAuth2AuthorizationEndpointFilter should be implemented as a Filter. The OAuth2AuthorizationRequestRedirectFilter in the spring-security-oauth2-client module is the client Filter that redirects to the Authorization Endpoint.

Implementation Requirements

• the Filter should process requests for the (default) path /oauth2/authorize
• authorizationRequestConverter should convert a valid Authorization Request to OAuth2AuthorizationRequest
• the RegisteredClientRepository Implement Client Registration Model / Repository #40 should be used to validate the client_id parameter
• the OAuth2AuthorizationService Implement Authorization Model / Service #43 should be used to persist the in-flight OAuth2Authorization
• the codeGenerator should be used to generate the authorization code parameter and it should also be stored in OAuth2Authorization.attributes for later validation in Implement Token Endpoint #67
• javadoc class and public methods
• Unit tests

Specification References

3.1. Authorization Endpoint
4.1. Authorization Code Grant
4.1.1. Authorization Request
4.1.2. Authorization Response

[paurav-munshi]

@jgrandja I would like to work on this issue. Please let me know if its good to start work on this ?

[jgrandja]

Thanks @paurav-munshi. The issue is yours.

[paurav-munshi]

@jgrandja

Can you please clarify few things ?

1. How will we handle the authentication part of the authorize endpoint ? Is is the scope of this issue ? If so then we will have to redirect the user-agent to a login endpoint

2. Once the code is generation, it will be the responsibility of the filter to redirect the user-agent to the configured redirect-uri along with code in query string. Is this understanding correct ?

3. Is there any reason why code needs to be in OAuth2Authorization.attributes and not have its own class like OAuth2AccessToken ? I am asking because code itself will need additional attributes like creating time and access counter at a minimum. So a separate class or representation by OAuth2AccesToken should be better.

[jgrandja]

@paurav-munshi

How will we handle the authentication part of the authorize endpoint ?

The default security configuration (WebSecurityConfigurerAdapter) will be configured with authenticated() for the /oauth2/authorize endpoint, so the existing features of Spring Security will be leveraged here - in a later story.

Once the code is generation, it will be the responsibility of the filter to redirect the user-agent to the configured redirect-uri along with code in query string. Is this understanding correct ?

Yes, that is correct. Take a look at how OAuth2AuthorizationRequestRedirectFilter (in spring-security-oauth2-client) redirects to the Authorization Endpoint. Also, please review 4.1.2. Authorization Response

Is there any reason why code needs to be in OAuth2Authorization.attributes and not have its own class like OAuth2AccessToken ?

OAuth2AccessToken is common across all grant flows. However, code is only applicable to the authorization_code grant. So I'd prefer to keep common attributes at the high-level of OAuth2Authorization and non-common attributes in OAuth2Authorization.attributes.

I am asking because code itself will need additional attributes like creating time and access counter at a minimum. So a separate class or representation by OAuth2AccesToken should be better.

Yes, great observation! This will come in a later story. We will need to expire code after a configured amount of time, as well, code should never be used more than once. We'll get there, but for now, just store code as a String in OAuth2Authorization.attributes.

[paurav-munshi]

@jgrandja

I have created a new pull request #76 for this issue. I request you to review it. I have not yet added the tests, but I think once you are fine with the code I will add javadoc and tests. Also there are a few assumptions and pointers on which I wish to have your inputs :-

Assumptions

• Several important components including converter, repository, authorization service, redirection strategy will be injected. Therefore I have not provided for their implementations here.
• Filter will be matched with the endpoint URI in outside configuration. Therefore filter is not checking the endpoint URI.

Not covered as they might make PR bigger

• Implementation of the converter
• In RegisteredClient make all Set to arrays or make getter methods return unmodifiable sets. Since we are using a builder, we can make them as arrays.
• Add validation for scope rules from spec
• Add validation for redirect uri rules from spec
• Add scope, redirect uri, state in OAuthAuthorization as they will be requried in token generation

I think we should either cover the missing validations from spec in this PR or create a new issue for the same.

Thanks,
Paurav.

[jgrandja]

@paurav-munshi I took a look at the PR and it's missing some key pieces of functionality required for this issue.

Several important components including converter, repository, authorization service, redirection strategy will be injected. Therefore I have not provided for their implementations here.

The Converter<HttpServletRequest, OAuth2AuthorizationRequest> authorizationRequestConverter is required as part of this implementation. Just like the OAuth2AuthorizationRequestRedirectFilter is responsible for producing the Authorization Request, this filter is responsible for consuming it.

The RedirectStrategy is also required. We can leverage what is available in Spring Security today, just like what's implemented in OAuth2AuthorizationRequestRedirectFilter - uses DefaultRedirectStrategy.

I also noticed the use of StringBuilder for building the authorization response, which is quite error prone. Please use UriComponentsBuilder instead.

Filter will be matched with the endpoint URI in outside configuration. Therefore filter is not checking the endpoint URI.

Spring Security Filter(s) typically match on a path first and then may additionally match on request parameters. As part of this issue

the Filter should process requests for the (default) path /oauth2/authorize

This request path should be the first check on whether this Filter will handle the request or not.
Please take a look at OAuth2AuthorizationRequestRedirectFilter, OAuth2LoginAuthenticationFilter and OAuth2AuthorizationCodeGrantFilter to see how things are implemented there. We are looking to align the implementations to "look-and-feel" the same as what's been built in Spring Security's OAuth support.

Before I do a detailed review, please add tests against the Filter to ensure the implementation is working as expected.

[paurav-munshi]

@jgrandja

I have created a new pull request #76 for this issue. I request you to review it. I have not yet added the tests, but I think once you are fine with the code I will add javadoc and tests. Also there are a few assumptions and pointers on which I wish to have your inputs

In my PR I have included some basic client id and redirect uri validations. But I am not sure if I have to include the client_id / scope / redirect_uri etc validations in this issue. There is confusion primarily because we do not have an issue for Resource Owner Authentication. As per spec in section 4.1.1 Authorization Request

The authorization server validates the request to ensure that all
required parameters are present and valid. If the request is valid,
the authorization server authenticates the resource owner and obtains
an authorization decision (by asking the resource owner or by
establishing approval via other means).

When a decision is established, the authorization server directs the
user-agent to the provided client redirection URI using an HTTP
redirection response, or by other means available to it via the
user-agent.

the request validation needs to happen before resource owner authentication. That means this issue will not be doing a bulk of validations. Can you please suggest what should be done with validations ? If we can identify which validation will be done a) before authentication b) before seeking user's permission c) before providing authorization code then it would be better. As per spec it seems only validation at this point in flow would be to check if resource owner gave the authorization consent or not.

Thanks,
Paurav.

[jgrandja]

The only validation required is what's specified in 4.1.1. Authorization Request.

To clarify this statement:

The authorization server validates the request to ensure that all
required parameters are present and valid. If the request is valid,
the authorization server authenticates the resource owner and obtains
an authorization decision (by asking the resource owner or by
establishing approval via other means)

The statement the authorization server authenticates the resource owner may cause confusion here as far as the order. The first thing that has to happen before an Authorization Request can be processed, is the resource owner must be authenticated first. This will happen outside of this issue. The Authorization Endpoint should expect that currentAuthentication.isAuthenticated() is true and then it would proceed to validate the authorization request parameters.

obtains an authorization decision will be implemented in #42

[dfcoffin]

@paurav-munshi @jgrandja I found when explaining the OAuth 2.0 Authorization Code flow that reviewing Section 4.1.2.1. Error Response and the description of the various possible error responses helped me understand what validation requirements must be met, prior to obtaining the resource owner's authentication and authorization decision.

The resource owner authentication and decision process are altered if the Authorization Code validation fails:

   If the request fails due to a missing, invalid, or mismatching
   redirection URI, or if the client identifier is missing or invalid,
   the authorization server SHOULD inform the resource owner of the
   error and MUST NOT automatically redirect the user-agent to the
   invalid redirection URI.

[paurav-munshi]

@jgrandja

I have incorporated the changes suggested by you and also added the test cases for the filter class. Please note that I have closed earlier PR #76 and opened a new PR #77 . So I request you to please review the new PR and ignore the old one. Sorry for the inconvenience if any.

I have applied following validations in the Filter :-

• Client Id should be mandatory
• Client Id should be registered
• Client Id should be configured with Authorization grant type
• Response type is mandaotry
• Response type should be code
• Redirect Uri should be specified if client has multiple redirect uris configured
• If supplied, redirect uri should be same as the one configured on client
• User should be authenticated
• Endpoint should be /oauth2/authorize