Disable device authorization flow by default #1498
Conversation
Disable device authorization grant by default. Such as, deviceAuthorizationEndpoint and deviceVerificationEndpoint
spring-projects-issues
added
the
status: waiting-for-triage
Disable device authorization grant by default. Such as, deviceAuthorizationEndpoint and deviceVerificationEndpoint
jgrandja
added
type: enhancement
| @@ -359,8 +403,6 @@ private Map<Class<? extends AbstractOAuth2Configurer>, AbstractOAuth2Configurer> | |||
| configurers.put(OAuth2TokenEndpointConfigurer.class, new OAuth2TokenEndpointConfigurer(this::postProcess)); | |||
| configurers.put(OAuth2TokenIntrospectionEndpointConfigurer.class, new OAuth2TokenIntrospectionEndpointConfigurer(this::postProcess)); | |||
| configurers.put(OAuth2TokenRevocationEndpointConfigurer.class, new OAuth2TokenRevocationEndpointConfigurer(this::postProcess)); | |||
| configurers.put(OAuth2DeviceAuthorizationEndpointConfigurer.class, new OAuth2DeviceAuthorizationEndpointConfigurer(this::postProcess)); | |||
There was a problem hiding this comment.
This change breaks (backward) compatibility since the feature is enabled by default. Instead of disabling by default, there needs to be a configurable way of disabling the feature.
There was a problem hiding this comment.
Do we need to provider a configuration(e.g. disableDeviceAuthorizationFlow) and default value is true? Any setting we can refer to?
There was a problem hiding this comment.
@jgrandja ，
Adding a configuration deviceAuthorizationEndpoint.enableDeviceAuthorizationEndpoint(true); for enable/disable device authorization flow. The default value of deviceAuthorizationEndpoint.enableDeviceAuthorizationEndpoint is true which enable by default for backward compatibility.
Provide a configuration "enableDeviceAuthorizationEndpoint" to support enable/disable device authorization grant. The default value of enableDeviceAuthorizationEndpoint is true for backward capability.
| @@ -161,6 +167,11 @@ public OAuth2DeviceAuthorizationEndpointConfigurer verificationUri(String verifi | |||
| return this; | |||
| } | |||
|
|
|||
| public OAuth2DeviceAuthorizationEndpointConfigurer enableDeviceAuthorizationEndpoint(boolean enableDeviceAuthorizationEndpoint) { | |||
There was a problem hiding this comment.
I'm not keen on exposing it this way. As well, we would need to provide the same to OAuth2DeviceVerificationEndpointConfigurer to be consistent.
I also think we should come up with a solution that allows other endpoints to be disabled if needed. I don't think a "flag" setting on all the configurers makes sense.
At this point, I don't have any suggestions but I'll give it some thought. Please give it some further thought s well.
There was a problem hiding this comment.
@gregecho Just circling back to this to see if you have come up with an alternative solution?
There was a problem hiding this comment.
@jgrandja , sorry about that. I didn't have a chance to discovery the alternative solution. Any ideas?
There was a problem hiding this comment.
@gregecho I haven't had time either to come up with a solution. I'm going to close this PR for now. When you have time and come up with an alternative solution, please propose it here first and then I can either re-open this PR or you can submit a new one. Thanks.
jgrandja
added
status: declined
|
Hi guys, Thought I would add my 2c regarding.
I second the notion regarding boolean flags, as the readability and granularity usually suffers greatly along this path. Instead this got me thinking of something along the lines of an "implicit" flag. Consider the scenario where a Then the value (I also suspect this approach would enable a path for aligning the |
Fix gh-1454, Disable device authorization grant by default. Such as, deviceAuthorizationEndpoint and deviceVerificationEndpoint