Allow non-authorization code requests to generate the sid field.

[leshalv]

No description provided.

[jgrandja]

@leshalv I'm trying to understand the reason for this change.

The sid claim is specified as follows:

OPTIONAL. Session ID - String identifier for a Session. This represents a Session of a User Agent or device for a logged-in End-User at an RP. Different sid values are used to identify distinct sessions at an OP. The sid value need only be unique in the context of a particular issuer. Its contents are opaque to the RP. Its syntax is the same as an OAuth 2.0 Client Identifier.

At the moment, Spring Authorization Server supports OpenID Connect Authentication using authorization_code flow only. So it doesn't make sense to add the sid claim to non-authorization code requests.

As an FYI, before submitting a PR, please log an issue first describing the proposed enhancement so we can discuss beforehand.

Please provide your reasoning on this proposed change.

[leshalv]

Current Implementation and Limitations

At present, the Spring Authorization Server supports OpenID Connect Authentication using the authorization_code flow only. The sid claim is optional, and adding it to non-authorization code requests might seem unnecessary at first glance. However, there are several compelling reasons to consider including the sid claim even in the current setup:

Reasons for Adding the sid Claim

1. Future-proofing for Additional Flows:

   • The inclusion of the sid claim now sets the stage for future support of other OpenID Connect flows, such as implicit or hybrid flows. This proactive step can simplify future updates and reduce the need for extensive modifications when additional flows are introduced.

2. Enhanced Session Management:

   • Even within the authorization_code flow, the sid claim can be beneficial for session management. It can help in tracking user sessions more effectively, providing a clearer picture of user activity and aiding in session-related operations such as logout and session revocation.

3. Consistency Across Implementations:

   • By including the sid claim, the Spring Authorization Server can align more closely with the OpenID Connect specification and other implementations that support multiple flows. This consistency can make it easier for developers familiar with OpenID Connect to work with the Spring Authorization Server.

4. Improved User Experience:

   • Enhanced session tracking can lead to a better user experience. For example, it can facilitate seamless single sign-on (SSO) experiences and provide users with more reliable session management, reducing instances of unexpected logouts or session conflicts.

5. Security Considerations:

   • The sid claim can also contribute to security enhancements. By clearly identifying user sessions, it becomes easier to detect and manage session hijacking attempts or other malicious activities. This additional layer of security can be valuable in protecting user data and ensuring secure authentication flows.

[jgrandja]

@leshalv

Even within the authorization_code flow, the sid claim can be beneficial for session management. It can help in tracking user sessions more effectively, providing a clearer picture of user activity and aiding in session-related operations such as logout and session revocation

I don't understand why this is listed since the sid claim is already included in the authorization_code flow.

By including the sid claim, the Spring Authorization Server can align more closely with the OpenID Connect specification and other implementations that support multiple flows. This consistency can make it easier for developers familiar with OpenID Connect to work with the Spring Authorization Server.

This comment is too general. What part of the current implementation does not align with the spec? Please be specific and reference the part of the spec that the current implementation does not align with.

The comments under "Improved User Experience" and "Security Considerations" are also too general and speculative. Again, if you see room for improvement, please be very specific and demonstrate there is an issue with the current implementation.

The inclusion of the sid claim now sets the stage for future support of other OpenID Connect flows, such as implicit or hybrid flows

FYI, there are no plans to support the implicit or hybrid flows.

Since the sid claim is used only in the authorization_code flow in the current implementation, I'm going to close this PR. If we decide to support additional flows, we can add the sid claim at that point.

[leshalv]

When using it, if you want to extend other authentication methods, such as implicit authorization, you also need sid and nonce, I don't think you have to judge whether it is authorization_code here, because the id_token itself can contain sid and nonce, which is convenient for better expansion and reduces duplicate boilerplate code. @jgrandja

[jgrandja]

if you want to extend other authentication methods, such as implicit authorization, you also need sid and nonce

You can register a custom OAuth2TokenCustomizer<JwtEncodingContext> @Bean for adding the sid or nonce claim