mirror of
https://github.com/goauthentik/authentik
synced 2026-04-25 17:15:26 +02:00
providers/oauth2: require client_secret on device_code exchange for confidential clients (#21700)
* providers/oauth2: require client_secret on device_code exchange for confidential clients
TokenParams.__post_init__ only ran the client_secret check for the
authorization_code and refresh_token grant types:
if self.grant_type in [GRANT_TYPE_AUTHORIZATION_CODE, GRANT_TYPE_REFRESH_TOKEN]:
if self.provider.client_type == ClientTypes.CONFIDENTIAL and not compare_digest(
self.provider.client_secret, self.client_secret,
):
raise TokenError("invalid_client")
The device_code path (__post_init_device_code) then looked up the
DeviceToken solely by device_code and issued an access token if one
matched. A caller that knows the client_id and has stolen a
device_code (e.g. via the standard phishing flow: attacker starts
device authorization, sends user_code to a victim, victim completes
authorization, attacker redeems the device_code) did not have to
prove ownership of the confidential client.
RFC 6749 Section 2.3.1 requires confidential clients to authenticate
to the token endpoint, and RFC 8628 Section 3.4 inherits that: the
device_code is bearer-shaped but not a substitute for client
credentials. Keycloak and Okta both enforce client_secret on the
device token exchange for confidential clients; we didn't.
Add GRANT_TYPE_DEVICE_CODE to the list so the existing compare_digest
check runs for it too. Public clients are unaffected (the guard is
gated on ClientTypes.CONFIDENTIAL). client_credentials/password keep
their own client-auth path in __post_init_client_credentials, which
also enforces the secret (and supports client assertion).
Fixes #20828
Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
* Apply suggestion from @BeryJu
Signed-off-by: Jens L. <jens@beryju.org>
* update tests
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
---------
Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
Signed-off-by: Jens L. <jens@beryju.org>
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
Co-authored-by: SAY-5 <SAY-5@users.noreply.github.com>
Co-authored-by: Jens L. <jens@beryju.org>
Co-authored-by: Jens Langhammer <jens@goauthentik.io>
This commit is contained in:
@@ -48,6 +48,7 @@ class TestTokenDeviceCode(OAuthTestCase):
|
||||
reverse("authentik_providers_oauth2:token"),
|
||||
data={
|
||||
"client_id": self.provider.client_id,
|
||||
"client_secret": self.provider.client_secret,
|
||||
"grant_type": GRANT_TYPE_DEVICE_CODE,
|
||||
},
|
||||
)
|
||||
@@ -66,6 +67,7 @@ class TestTokenDeviceCode(OAuthTestCase):
|
||||
reverse("authentik_providers_oauth2:token"),
|
||||
data={
|
||||
"client_id": self.provider.client_id,
|
||||
"client_secret": self.provider.client_secret,
|
||||
"grant_type": GRANT_TYPE_DEVICE_CODE,
|
||||
"device_code": device_token.device_code,
|
||||
},
|
||||
@@ -74,6 +76,26 @@ class TestTokenDeviceCode(OAuthTestCase):
|
||||
body = loads(res.content.decode())
|
||||
self.assertEqual(body["error"], "authorization_pending")
|
||||
|
||||
def test_code_no_auth(self):
|
||||
"""Test code with user"""
|
||||
device_token = DeviceToken.objects.create(
|
||||
provider=self.provider,
|
||||
user_code=generate_code_fixed_length(),
|
||||
device_code=generate_id(),
|
||||
user=self.user,
|
||||
)
|
||||
res = self.client.post(
|
||||
reverse("authentik_providers_oauth2:token"),
|
||||
data={
|
||||
"client_id": self.provider.client_id,
|
||||
"grant_type": GRANT_TYPE_DEVICE_CODE,
|
||||
"device_code": device_token.device_code,
|
||||
},
|
||||
)
|
||||
self.assertEqual(res.status_code, 400)
|
||||
body = loads(res.content.decode())
|
||||
self.assertEqual(body["error"], "invalid_client")
|
||||
|
||||
def test_code(self):
|
||||
"""Test code with user"""
|
||||
device_token = DeviceToken.objects.create(
|
||||
@@ -86,6 +108,7 @@ class TestTokenDeviceCode(OAuthTestCase):
|
||||
reverse("authentik_providers_oauth2:token"),
|
||||
data={
|
||||
"client_id": self.provider.client_id,
|
||||
"client_secret": self.provider.client_secret,
|
||||
"grant_type": GRANT_TYPE_DEVICE_CODE,
|
||||
"device_code": device_token.device_code,
|
||||
},
|
||||
@@ -105,6 +128,7 @@ class TestTokenDeviceCode(OAuthTestCase):
|
||||
reverse("authentik_providers_oauth2:token"),
|
||||
data={
|
||||
"client_id": self.provider.client_id,
|
||||
"client_secret": self.provider.client_secret,
|
||||
"grant_type": GRANT_TYPE_DEVICE_CODE,
|
||||
"device_code": device_token.device_code,
|
||||
"scope": f"{SCOPE_OPENID} {SCOPE_OPENID_EMAIL} invalid",
|
||||
|
||||
@@ -165,7 +165,15 @@ class TokenParams:
|
||||
raise TokenError("invalid_grant")
|
||||
|
||||
def __post_init__(self, raw_code: str, raw_token: str, request: HttpRequest):
|
||||
if self.grant_type in [GRANT_TYPE_AUTHORIZATION_CODE, GRANT_TYPE_REFRESH_TOKEN]:
|
||||
# Confidential clients MUST authenticate to the token endpoint per
|
||||
# RFC 6749 §2.3.1. The device code grant (RFC 8628 §3.4) inherits
|
||||
# that requirement - the device_code alone is not a substitute for
|
||||
# client credentials.
|
||||
if self.grant_type in [
|
||||
GRANT_TYPE_AUTHORIZATION_CODE,
|
||||
GRANT_TYPE_REFRESH_TOKEN,
|
||||
GRANT_TYPE_DEVICE_CODE,
|
||||
]:
|
||||
if self.provider.client_type == ClientTypes.CONFIDENTIAL and not compare_digest(
|
||||
self.provider.client_secret, self.client_secret
|
||||
):
|
||||
|
||||
Reference in New Issue
Block a user