Skip to content

Improve OAuth 2.0 Device Authorization Grant #1116

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
4 tasks done
sjohnr opened this issue Mar 10, 2023 · 13 comments
Closed
4 tasks done

Improve OAuth 2.0 Device Authorization Grant #1116

sjohnr opened this issue Mar 10, 2023 · 13 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@sjohnr sjohnr added the type: enhancement A general enhancement label Mar 10, 2023
@sjohnr sjohnr added this to the 1.1.0-RC1 milestone Mar 10, 2023
@sjohnr sjohnr self-assigned this Mar 10, 2023
@svrakitin
Copy link

svrakitin commented Mar 14, 2023

Hey @sjohnr!

I noticed that PublicClientAuthenticationConverter wasn't extended for Device Authorization Grant, which makes it work only with confidential clients (with a secret), while device flow is supposed to work with public clients (without a secret).

It also doesn't provide an ID token in the token response even if openid scope is there. Is it by design?

@sjohnr
Copy link
Contributor Author

sjohnr commented Mar 14, 2023

Hi @svrakitin! I appreciate the questions.

I noticed that PublicClientAuthenticationConverter wasn't extended for Device Authorization Grant, which makes it work only with confidential clients (with a secret), while device flow is supposed to work with public clients (without a secret).

Whether the authorization server supports public clients (and how) is not really specified in the spec, only that device clients are treated the same as public clients (RFC 8628, Section 5.6). We will continue to make the same recommendation for device grant as for authorization code, and so we didn't include support for public clients. I've added the device-client sample as a minimal example of using a confidential client to proxy the flow for a SPA or native app.

It also doesn't provide an ID token in the token response even if openid scope is there. Is it by design?

The OAuth 2.0 Device Authorization Grant (RFC 8628) is not connected to OpenID Connect 1.0, does not mention the openid scope, and does not provide login functionality. Are you referring to a different specification?

@svrakitin
Copy link

svrakitin commented Mar 14, 2023

Whether the authorization server supports public clients (and how) is not really specified in the spec, only that device clients are treated the same as public clients (RFC 8628, Section 5.6). We will continue to make #297 for device grant as for authorization code, and so we didn't include support for public clients. I've added the device-client sample as a minimal example of using a confidential client to proxy the flow for a SPA or native app.

Device Authorization Grant assumes a public client, as device (e.g. smart TV, POS terminal or CLI) can't really contain credentials. You linked the correct section (RFC 8628, Section 5.6). This would be similar to the browser-only clients. Recommendation to use a confidential client doesn't apply here as those shouldn't be even supported.

The OAuth 2.0 Device Authorization Grant (RFC 8628) is not connected to OpenID Connect 1.0, does not mention the openid scope, and does not provide login functionality. Are you referring to a different specification?

It is a convention across vendors (at least Okta, Auth0, Azure) to treat device flow same way as authorization code flow when openid scope is used. The only difference is that the device can't handle the callback in the browser.

@sjohnr
Copy link
Contributor Author

sjohnr commented Mar 14, 2023

Thanks for your response, @svrakitin.

Recommendation to use a confidential client doesn't apply here as those shouldn't be even supported.

According to Section 3.1 Device Authorization Request, the client authentication requirements of RFC 6749, Section 3.2.1 apply, meaning confidential clients are supported. As mentioned in the linked issue, the backend-for-frontend pattern provides a confidential client, and device clients can proxy requests through a BFF similar to browser-based apps. The BFF can allow anonymous requests, and would be responsible for protecting the backend from abuse.

On the contrary, since public clients don't have credentials, there's not a clear way to alternatively protect the Device Authorization Endpoint and it is not described in the spec. Do you have a suggestion for how to securely support public clients? I think allowing anonymous access will only be possible through customization, and the application would assume any associated risk(s).

It is a convention across vendors (at least Okta, Auth0, Azure) to treat device flow same way as authorization code flow when openid scope is used.

Thanks for providing some links. I'll review those when time allows. However, I think this would be a customization that needs to be made in the application as it is not in the spec. The goal of this project is to implement the specifications to the greatest extent possible. Feel free to log a separate issue with this suggestion, but I don't anticipate adding direct support for this.

@svrakitin
Copy link

svrakitin commented Mar 14, 2023

Thanks for linking the section @sjohnr, TIL. I wonder what would be the use-case for confidential clients going through Device Authorization flow. Authorization Code flow at least provides some basic protection via exact match on redirect URIs. Here, it is enough for credentials to be compromised.

What do we want to protect the endpoint from? Are we concerned about unauthenticated traffic? This would be the same case as Dynamic Client Registration, where you expect unauthenticated requests. Endpoint protection here is a responsibility of an application developer.

My suggestion is to enable public clients via customization option if you believe this is not secure by default. Otherwise, this doesn't cover the primary purpose of Device Authorization Grant - enable headless clients to act on behalf of users.

I am happy to contribute.


I also noticed that we store the client principal under authorization attribute when handling device authorization request, which seems unnecessary:

Everything works without that and avoids serializing a registered client contained in that principal.

@sjohnr
Copy link
Contributor Author

sjohnr commented Mar 15, 2023

Hi @svrakitin.

I wonder what would be the use-case for confidential clients going through Device Authorization flow.

The feedback you're providing is great. However, it would be helpful if you could read up on the links provided in gh-297 on BFF, as I'm not sure we're thinking of the same use of a confidential client in the conversation so far.

My suggestion is to enable public clients via customization option if you believe this is not secure by default.

Please log an issue for this so it can be discussed first.

I am happy to contribute.

Awesome!

Everything works without that and avoids serializing a registered client contained in that principal.

I'll keep that in mind! My initial goal was to be consistent with the authorization code implementation.

@jgrandja
Copy link
Collaborator

@svrakitin

I also noticed that we store the client principal under authorization attribute when handling device authorization request, which seems unnecessary

Yes, the client Principal is initially saved in the OAuth2Authorization at the start of the flow, however, when the user-interaction steps are initiated, the End-User Principal is updated in OAuth2Authorization replacing the client Principal.

My suggestion is to enable public clients via customization option if you believe this is not secure by default. Otherwise, this doesn't cover the primary purpose of Device Authorization Grant - enable headless clients to act on behalf of users.

@sjohnr and I are currently discussing various options on providing support for Public Clients. It will likely be via customizations by the consuming application. We'll be sure to include you when we have more information so we can hear your feedback.

@svrakitin
Copy link

svrakitin commented Mar 27, 2023

@jgrandja

Yes, the client Principal is initially saved in the OAuth2Authorization at the start of the flow, however, when the user-interaction steps are initiated, the End-User Principal is updated in OAuth2Authorization replacing the client Principal.

I just had to allowlist OAuth2ClientAuthenticationToken class when serializing attributes before persisting them to the database. It is currently missing from https://github.com/spring-projects/spring-authorization-server/blob/main/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/jackson2/OAuth2AuthorizationServerJackson2Module.java#L74, which I use as reference for my own implementation.

@jgrandja
Copy link
Collaborator

@svrakitin

I just had to allowlist OAuth2ClientAuthenticationToken class when serializing attributes

I see the issue now. This has been fixed in 5b690df

@jgrandja
Copy link
Collaborator

@svrakitin

My suggestion is to enable public clients via customization option if you believe this is not secure by default. Otherwise, this doesn't cover the primary purpose of Device Authorization Grant - enable headless clients to act on behalf of users.

It's possible to allow public clients for device grant via customization of OAuth2AuthorizationServerConfigurer.clientAuthentication(). See this branch/commit for full working sample. We will likely merge this with our current sample and document it in the reference manual as well.

@svrakitin
Copy link

@jgrandja

Thanks. That's how I've implemented it as well, it's just something I'd like to have out of the box. :)

Why client-specific consent settings are no longer taken into account when deciding whether to ask for consent? This line was dropped 1354ca4#diff-286505367c851db9d5dd851f4a0773459e01d93338778b453a7c067ed193e940L185

@jgrandja
Copy link
Collaborator

@svrakitin

Why client-specific consent settings are no longer taken into account when deciding whether to ask for consent?

As per section 5.4 Remote Phishing:

it is RECOMMENDED to
inform the user that they are authorizing a device during the user-
interaction step (see Section 3.3) and to confirm that the device is
in their possession. The authorization server SHOULD display
information about the device so that the user could notice if a
software client was attempting to impersonate a hardware device.

The default for ClientSettings.isRequireAuthorizationConsent() is false, so it would bypass the consent screen in the default configuration, hence the removal of that check.

Is this a concern for you? If so, do you have a suggested improvement?

@svrakitin
Copy link

@jgrandja Thanks. It is fine, I was just curious, because it wasn't consistent with Authorization Code flow.

@sjohnr sjohnr closed this as completed Apr 12, 2023
sjohnr pushed a commit that referenced this issue May 2, 2023
tjholmes66 added a commit to tjholmes66/spring-authorization-server that referenced this issue Oct 2, 2023
* Update to io.spring.javaformat:spring-javaformat-checkstyle:0.0.35

Closes spring-projectsgh-1089

* Update to jackson-bom:2.14.2

Closes spring-projectsgh-1090

* Update to junit-jupiter:5.9.2

Closes spring-projectsgh-1091

* Release 1.0.1

* Next Development Version

* Update to Spring Security 6.1.0-M1

Closes spring-projectsgh-1093

* Update to nimbus-jose-jwt:9.30.2

Closes spring-projectsgh-1094

* Update to assertj-core:3.24.2

Closes spring-projectsgh-1095

* Update to mockito-core:4.11.0

Closes spring-projectsgh-1096

* Release 1.1.0-M1

* Next Development Version

* Add user property to deploy_docs workflow

* Fix broken support link

Closes spring-projectsgh-1092

* Fix client secret encoding when client dynamically registered

Closes spring-projectsgh-1056

* Polish spring-projectsgh-1056

* Allow PasswordEncoder to be configured in OidcClientRegistrationAuthenticationProvider

Issue spring-projectsgh-1056

* Upgrade client secret when available

Closes spring-projectsgh-1099

* Polish spring-projectsgh-1105

* Add support for OAuth 2.0 Device Authorization Grant

Closes spring-projectsgh-44

* Switch to spring-security SNAPSHOT dependencies

Issue spring-projectsgh-44

* Use spring-security 6.1 in snapshot tests

Issue spring-projectsgh-1106

* Update to actions/checkout@v3

Closes spring-projectsgh-1117

* Use spring-io/spring-gradle-build-action

Closes spring-projectsgh-1120

* Use spring-io/spring-gradle-build-action

Closes spring-projectsgh-1120

* Revert accidental change in versions

Issue spring-projectsgh-1120

* Polish spring-projectsgh-1106

* Update to Spring Framework 6.0.7

Closes spring-projectsgh-1130

* Update to Spring Security 1.1.0-M2

Closes spring-projectsgh-1131

* Update to nimbus-jose-jwt:9.31

Closes spring-projectsgh-1132

* Update to Spring Framework 6.0.7 in buildSrc

Issue spring-projectsgh-1130

* Release 1.1.0-M2

* Next Development Version

* Polish spring-projectsgh-1106 Device Authorization Grant

* Avoid persisting client principal in device authorization request

Issue spring-projectsgh-1106

* Polish spring-projectsgh-1068

Issue spring-projectsgh-1077

* Add OidcLogoutAuthenticationToken.isPrincipalAuthenticated()

Issue spring-projectsgh-1077

* Ensure ID Token is active before processing logout request

Issue spring-projectsgh-1077

* Allow localhost in redirect_uri

Closes spring-projectsgh-651

* Fix refresh token error code INVALID_CLIENT to INVALID_GRANT

Closes spring-projectsgh-1139

* Do not require authorizationRequest for device grant

Issue spring-projectsgh-1127

* Add tests for OAuth 2.0 Device Authorization Grant

This commit adds tests for the following components:
* AuthenticationConverters
* AuthenticationProviders
* Endpoint Filters

Issue spring-projectsgh-44
Closes spring-projectsgh-1127

* JDBC device_code authorization

Issue spring-projectsgh-1156

* Polish spring-projectsgh-1143

* Add tests and update examples in docs

Closes spring-projectsgh-1156

* Polish ref-doc

Issue spring-projectsgh-1156

* Add customization to support public clients for device grant

Issue spring-projectsgh-1157

* Polish samples

Closes spring-projectsgh-1157

* Add documentation for OAuth 2.0 Device Authorization Grant

Closes spring-projectsgh-1158

* Polish spring-projectsgh-1127

* Polish spring-projectsgh-1158

* Add documentation for OpenID Connect 1.0 Logout Endpoint

Closes spring-projectsgh-1069

* Update Stack Overflow tag to spring-authorization-server

* Update to Spring Framework 5.3.27

Closes spring-projectsgh-1162

* Update to Spring Security 5.8.3

Closes spring-projectsgh-1163

* Update to io.spring.javaformat:spring-javaformat-checkstyle:0.0.38

Closes spring-projectsgh-1164

* Update to Spring Framework 6.0.8

Closes spring-projectsgh-1165

* Update to Spring Security 6.0.3

Closes spring-projectsgh-1166

* Update to io.spring.javaformat:spring-javaformat-checkstyle:0.0.38

Closes spring-projectsgh-1167

* Update to Spring Framework 6.0.8

Closes spring-projectsgh-1168

* Update to Spring Security 6.1.0-RC1

Closes spring-projectsgh-1169

* Update to io.spring.javaformat:spring-javaformat-checkstyle:0.0.38

Closes spring-projectsgh-1170

* Update to json-path:2.8.0

Closes spring-projectsgh-1171

* Release 0.4.2

* Next Development Version

* Release 1.0.2

* Next Development Version

* Release 1.1.0-RC1

* Next Development Version

* Update to org.jfrog.buildinfo:build-info-extractor-gradle:4.29.0

Closes spring-projectsgh-1175

* Apply ArtifactoryPlugin to SpringRootProjectPlugin

Closes spring-projectsgh-1177

* Fix artifact build properties for Artifactory

- Apply SpringArtifactoryPlugin in SpringRootProjectPlugin (which applies ArtifactoryPlugin)
- In SpringArtifactoryPlugin don't set publication if MavenPublishPlugin is not applied

Closes spring-projectsgh-1179

* Add test for dynamic client registration with custom metadata

Issue spring-projectsgh-1172

* Add logout success page to default client sample

Sample client (located in 'samples/messages-client' directory) is configured with a custom logout success page where
the end-user is redirected to upon successful logout action.

Fixes spring-projectsgh-1142

* Add sample featured-authorizationserver

Issue spring-projectsgh-1189

* Merge custom-consent-authorizationserver into featured-authorizationserver

Issue spring-projectsgh-1189

* Merge federated-identity-authorizationserver into featured-authorizationserver

Issue spring-projectsgh-1189

* Update io.spring.ge.conventions plugin to 0.0.13

Closes spring-projectsgh-1190

* Update spring-asciidoctor-backends to 0.0.5

Closes spring-projectsgh-1192

* Merge device-grant-authorizationserver into featured-authorizationserver

Issue spring-projectsgh-1189

* Merge device-client into messages-client

Issue spring-projectsgh-1189

* Use custom consent page for device code flow

Issue spring-projectsgh-1189

* Use current authentication for device authorization

Issue spring-projectsgh-1189

* Reuse error handling

Issue spring-projectsgh-1189

* Handle web client response error

Issue spring-projectsgh-1189

* Update @SInCE

* Rename featured-authorizationserver to demo-authorizationserver

Issue spring-projectsgh-1189

* Rename messages-client to demo-client

Issue spring-projectsgh-1189

* Update sample README

Issue spring-projectsgh-1189

* Add integration tests for device grant

Issue spring-projectsgh-1116

* Update web ui design for demo-client

Issue spring-projectsgh-1196

* Polish web ui design for demo-client

Issue spring-projectsgh-1196

* Update web ui design for demo-authorizationserver

Issue spring-projectsgh-1196

* Polish web ui design for demo-client

Issue spring-projectsgh-1196

* Polish demo sample

Issue spring-projectsgh-1189

* Update to Spring Boot 3.1.0-RC1

Closes spring-projectsgh-1198

* Refresh Getting Started example

Closes spring-projectsgh-1186

* Use Spring Boot starter in samples

Closes spring-projectsgh-1187

* Invalidate tokens previously issued when code is reused

Closes spring-projectsgh-1152

* Polish spring-projectsgh-1152

* Add How-to: Authenticate using Social Login

Closes spring-projectsgh-538

* Add How-to: Authenticate using a Single Page Application with PKCE

Closes spring-projectsgh-539

* Hash the sid claim in the ID Token

Closes spring-projectsgh-1207

* Simplified federated login in demo sample

Closes spring-projectsgh-1208

* Polish spring-projectsgh-1186

* Polish spring-projectsgh-538

* Remove FederatedIdentityConfigurer from demo sample

Issue spring-projectsgh-1208

* Update exception handling config in ref-doc

Closes spring-projectsgh-1205

* Update exception handling config in samples

Closes spring-projectsgh-1204

* Polish ref-doc

Issue spring-projectsgh-1205

* Polish tests

* Add How-to: Implement an Extension Authorization Grant Type

Closes spring-projectsgh-686

* Update to Spring Framework 6.0.9

Closes spring-projectsgh-1213

* Update to Spring Security 6.1.0

Closes spring-projectsgh-1214

* Update to jackson-bom 2.15.0

Closes spring-projectsgh-1215

* Update to junit-jupiter 5.9.3

Closes spring-projectsgh-1216

* Release 1.1.0

* Next Development Version

* Revert serialVersionUID to 0.4.0

Closes spring-projectsgh-1218

* Revert serialVersionUID to 1.0.0

Closes spring-projectsgh-1219

* Revert serialVersionUID to 1.1.0

Closes spring-projectsgh-1220

* Exclude project dependency from spring-boot-dependencies

Closes spring-projectsgh-1228

* Update to Spring Boot 3.1.0

* Update com.gradle.enterprise plugin to 3.13.3

Closes spring-projectsgh-1234
Issue spring-projectsgh-1231

* Prepare for automated validation scripts

See https://github.com/gradle/gradle-enterprise-build-validation-scripts/blob/main/Gradle.md

Issue spring-projectsgh-1231

* ID Token contains sid claim after refresh_token grant

Closes spring-projectsgh-1224

* Use substring instead of replaceFirst in OAuth2AuthorizationConsent

Closes spring-projectsgh-1222

* Validate authorized principal instead of sub during logout

Closes spring-projectsgh-1235

* Revert "Prepare for automated validation scripts"

This reverts commit ece9f10.

Issue spring-projectsgh-1231

* Add debug log entries

Closes spring-projectsgh-1245
Closes spring-projectsgh-1246
Closes spring-projectsgh-1247
Closes spring-projectsgh-1248

* Polish additional logging

Issue spring-projectsgh-1245, spring-projectsgh-1246, spring-projectsgh-1247, spring-projectsgh-1248

* Enable caching of :asciidoctor gradle task

* Use direct code import

Issue spring-projectsgh-1231

* Next Minor Version

* Fix NPE on access token in OAuth2AuthorizationCodeAuthenticationProvider

Closes spring-projectsgh-1233

* Polish spring-projectsgh-1233

* Fix to save all values for multi-valued request parameters

Fixes spring-projectsgh-1250

* Polish spring-projectsgh-1252

* Fix to save all values for multi-valued device grant parameters

Fixes spring-projectsgh-1269

* Polish spring-projectsgh-1252

* Update to Spring Framework 5.3.28

Closes spring-projectsgh-1272

* Update to Spring Security 5.8.4

Closes spring-projectsgh-1273

* Update to jackson-bom 2.14.3

Closes spring-projectsgh-1274

* Update to Spring Framework 6.0.10

Closes spring-projectsgh-1276

* Update to Spring Security 6.0.4

Closes spring-projectsgh-1277

* Update to Spring Framework 6.0.10

Closes spring-projectsgh-1278

* Update to Spring Security 6.1.1

Closes spring-projectsgh-1279

* Update to junit-jupiter 5.9.3

Closes spring-projectsgh-1280

* Update to junit-jupiter 5.9.3

Closes spring-projectsgh-1281

* Update to jackson-bom 2.15.2

Closes spring-projectsgh-1282

* Update feature planning section to using GitHub Projects

* Release 1.1.1

* Next Development Version

* Fix generating ID token with null sid when refresh_token grant

Closes spring-projectsgh-1283

* Polish spring-projectsgh-1289

* Fix NPE in DefaultErrorController

Closes spring-projectsgh-1286

* Migrate docs to Antora

Issue spring-projectsgh-1295

* Polish Antora migration

Issue spring-projectsgh-1292
Closes spring-projectsgh-1295

* Remove unused antora-playbook.yml

* Replaces 'install' with 'publishToMavenLocal' command in README

* Adds how-to guide on adding authorities to access tokens

Closes spring-projectsgh-542

* Polish spring-projectsgh-1264

* Update order of guides in nav.adoc

* Fix userCode validation

Issue spring-projectsgh-44

* Polish spring-projectsgh-1309

* Add Revved up by Gradle Enterprise badge

* Move badges to header

This is similar to Spring Boot:
  https://github.com/spring-projects/spring-boot/blob/main/README.adoc

* Fix workflow status link

* Fix samples test suite execution and failing tests

Closes spring-projectsgh-1324

* Polish spring-projectsgh-1325

* Move deploy-docs.yml to correct folder

Issue spring-projectsgh-1295

* Remove manual list of guides

Issue spring-projectsgh-1295

* Remove reference to gh-pages

Issue spring-projectsgh-1295

* Update to Spring Framework 6.0.11

Closes spring-projectsgh-1338

* Update to Spring Security 6.1.2

Closes spring-projectsgh-1339

* Update to org.hsqldb:hsqldb 2.7.2

Closes spring-projectsgh-1340

* Release 1.1.2

* Next Development Version

* Adds ability to inject custom metadata at client registration

Closes spring-projectsgh-1172

* Polish spring-projectsgh-1326

* Adds dynamic client registration how-to guide

Closes spring-projectsgh-647

* Polish spring-projectsgh-1320

* Add code challenge methods for oidc provider configuration response

Closes spring-projectsgh-1302

* Update to Spring Framework 6.1.0-M5

Closes spring-projectsgh-1364

* Update to Spring Security 6.2.0-M3

Closes spring-projectsgh-1365

* Update to nimbus-jose-jwt 9.35

Closes spring-projectsgh-1366

* Update to junit-jupiter 5.10.0

Closes spring-projectsgh-1367

* Update to okhttp 4.11.0

Closes spring-projectsgh-1368

* Release 1.2.0-M1

* Next Development Version

---------

Co-authored-by: Joe Grandja <[email protected]>
Co-authored-by: Siva Kumar Edupuganti <[email protected]>
Co-authored-by: Yuta Saito <[email protected]>
Co-authored-by: Shannon Pamperl <[email protected]>
Co-authored-by: Steve Riesenberg <[email protected]>
Co-authored-by: HuiYeong <[email protected]>
Co-authored-by: Xu Xiaowei <[email protected]>
Co-authored-by: Janne Valkealahti <[email protected]>
Co-authored-by: Dmitriy Dubson <[email protected]>
Co-authored-by: neochae <[email protected]>
Co-authored-by: heartape <[email protected]>
Co-authored-by: Dejan Varmedja <[email protected]>
Co-authored-by: Jerome Prinet <[email protected]>
Co-authored-by: Pavel Efros <[email protected]>
Co-authored-by: Martin Lindström <[email protected]>
Co-authored-by: cbilodeau <[email protected]>
Co-authored-by: Rob Winch <[email protected]>
Co-authored-by: Dmitriy Dubson <[email protected]>
Co-authored-by: Martin Bogusz <[email protected]>
Co-authored-by: Eric Haag <[email protected]>
Co-authored-by: Tuxzx <[email protected]>
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

3 participants