providers/oauth2: inconsistent client secret generation (#5241)
* use simpler char set for client secret Signed-off-by: Jens Langhammer <jens@goauthentik.io> * also adjust radius Signed-off-by: Jens Langhammer <jens@goauthentik.io> * use similar logic in web to generate ids and secrets Signed-off-by: Jens Langhammer <jens@goauthentik.io> * dont use math.random Signed-off-by: Jens Langhammer <jens@goauthentik.io> --------- Signed-off-by: Jens Langhammer <jens@goauthentik.io>
This commit is contained in:
parent
7841720acf
commit
6a74fa11c6
|
@ -3,6 +3,8 @@
|
|||
import django.utils.timezone
|
||||
from django.db import migrations, models
|
||||
|
||||
import authentik.providers.oauth2.models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
dependencies = [
|
||||
|
@ -37,4 +39,14 @@ class Migration(migrations.Migration):
|
|||
),
|
||||
preserve_default=False,
|
||||
),
|
||||
migrations.AlterField(
|
||||
model_name="oauth2provider",
|
||||
name="client_secret",
|
||||
field=models.CharField(
|
||||
blank=True,
|
||||
default=authentik.providers.oauth2.models.generate_client_secret,
|
||||
max_length=255,
|
||||
verbose_name="Client Secret",
|
||||
),
|
||||
),
|
||||
]
|
||||
|
|
|
@ -27,6 +27,11 @@ from authentik.providers.oauth2.id_token import IDToken, SubModes
|
|||
from authentik.sources.oauth.models import OAuthSource
|
||||
|
||||
|
||||
def generate_client_secret() -> str:
|
||||
"""Generate client secret with adequate length"""
|
||||
return generate_id(128)
|
||||
|
||||
|
||||
class ClientTypes(models.TextChoices):
|
||||
"""Confidential clients are capable of maintaining the confidentiality
|
||||
of their credentials. Public clients are incapable."""
|
||||
|
@ -132,7 +137,7 @@ class OAuth2Provider(Provider):
|
|||
max_length=255,
|
||||
blank=True,
|
||||
verbose_name=_("Client Secret"),
|
||||
default=generate_key,
|
||||
default=generate_client_secret,
|
||||
)
|
||||
redirect_uris = models.TextField(
|
||||
default="",
|
||||
|
|
|
@ -7,7 +7,6 @@ from rest_framework.test import APITestCase
|
|||
from authentik.blueprints.tests import apply_blueprint
|
||||
from authentik.core.models import Application
|
||||
from authentik.core.tests.utils import create_test_admin_user, create_test_flow
|
||||
from authentik.lib.generators import generate_id, generate_key
|
||||
from authentik.providers.oauth2.models import OAuth2Provider, ScopeMapping
|
||||
|
||||
|
||||
|
@ -18,8 +17,6 @@ class TestAPI(APITestCase):
|
|||
def setUp(self) -> None:
|
||||
self.provider: OAuth2Provider = OAuth2Provider.objects.create(
|
||||
name="test",
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://testserver",
|
||||
)
|
||||
|
|
|
@ -9,7 +9,7 @@ from authentik.core.models import Application
|
|||
from authentik.core.tests.utils import create_test_admin_user, create_test_flow
|
||||
from authentik.events.models import Event, EventAction
|
||||
from authentik.flows.challenge import ChallengeTypes
|
||||
from authentik.lib.generators import generate_id, generate_key
|
||||
from authentik.lib.generators import generate_id
|
||||
from authentik.lib.utils.time import timedelta_from_string
|
||||
from authentik.providers.oauth2.constants import TOKEN_TYPE
|
||||
from authentik.providers.oauth2.errors import AuthorizeError, ClientIdError, RedirectUriError
|
||||
|
@ -298,7 +298,6 @@ class TestAuthorize(OAuthTestCase):
|
|||
provider: OAuth2Provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id="test",
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=flow,
|
||||
redirect_uris="http://localhost",
|
||||
signing_key=self.keypair,
|
||||
|
@ -361,7 +360,6 @@ class TestAuthorize(OAuthTestCase):
|
|||
provider: OAuth2Provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id="test",
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=flow,
|
||||
redirect_uris="http://localhost",
|
||||
signing_key=self.keypair,
|
||||
|
@ -417,7 +415,6 @@ class TestAuthorize(OAuthTestCase):
|
|||
provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=flow,
|
||||
redirect_uris="http://localhost",
|
||||
signing_key=self.keypair,
|
||||
|
@ -467,7 +464,6 @@ class TestAuthorize(OAuthTestCase):
|
|||
provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=flow,
|
||||
redirect_uris="http://localhost",
|
||||
signing_key=self.keypair,
|
||||
|
|
|
@ -8,7 +8,7 @@ from django.utils import timezone
|
|||
|
||||
from authentik.core.models import Application
|
||||
from authentik.core.tests.utils import create_test_admin_user, create_test_cert, create_test_flow
|
||||
from authentik.lib.generators import generate_id, generate_key
|
||||
from authentik.lib.generators import generate_id
|
||||
from authentik.providers.oauth2.constants import ACR_AUTHENTIK_DEFAULT
|
||||
from authentik.providers.oauth2.models import AccessToken, IDToken, OAuth2Provider, RefreshToken
|
||||
from authentik.providers.oauth2.tests.utils import OAuthTestCase
|
||||
|
@ -21,8 +21,6 @@ class TesOAuth2Introspection(OAuthTestCase):
|
|||
super().setUp()
|
||||
self.provider: OAuth2Provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="",
|
||||
signing_key=create_test_cert(),
|
||||
|
|
|
@ -8,7 +8,7 @@ from django.utils import timezone
|
|||
|
||||
from authentik.core.models import Application
|
||||
from authentik.core.tests.utils import create_test_admin_user, create_test_cert, create_test_flow
|
||||
from authentik.lib.generators import generate_id, generate_key
|
||||
from authentik.lib.generators import generate_id
|
||||
from authentik.providers.oauth2.models import AccessToken, IDToken, OAuth2Provider, RefreshToken
|
||||
from authentik.providers.oauth2.tests.utils import OAuthTestCase
|
||||
|
||||
|
@ -20,8 +20,6 @@ class TesOAuth2Revoke(OAuthTestCase):
|
|||
super().setUp()
|
||||
self.provider: OAuth2Provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="",
|
||||
signing_key=create_test_cert(),
|
||||
|
|
|
@ -38,8 +38,6 @@ class TestToken(OAuthTestCase):
|
|||
"""test request param"""
|
||||
provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://TestServer",
|
||||
signing_key=self.keypair,
|
||||
|
@ -67,8 +65,6 @@ class TestToken(OAuthTestCase):
|
|||
"""test request param"""
|
||||
provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://testserver",
|
||||
signing_key=self.keypair,
|
||||
|
@ -90,8 +86,6 @@ class TestToken(OAuthTestCase):
|
|||
"""test request param"""
|
||||
provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://local.invalid",
|
||||
signing_key=self.keypair,
|
||||
|
@ -120,8 +114,6 @@ class TestToken(OAuthTestCase):
|
|||
"""test request param"""
|
||||
provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://local.invalid",
|
||||
signing_key=self.keypair,
|
||||
|
@ -163,8 +155,6 @@ class TestToken(OAuthTestCase):
|
|||
"""test request param"""
|
||||
provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://local.invalid",
|
||||
signing_key=self.keypair,
|
||||
|
@ -215,8 +205,6 @@ class TestToken(OAuthTestCase):
|
|||
"""test request param"""
|
||||
provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://local.invalid",
|
||||
signing_key=self.keypair,
|
||||
|
@ -263,8 +251,6 @@ class TestToken(OAuthTestCase):
|
|||
"""test request param"""
|
||||
provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://testserver",
|
||||
signing_key=self.keypair,
|
||||
|
|
|
@ -8,7 +8,6 @@ from jwt import decode
|
|||
from authentik.blueprints.tests import apply_blueprint
|
||||
from authentik.core.models import USER_ATTRIBUTE_SA, Application, Group, Token, TokenIntents
|
||||
from authentik.core.tests.utils import create_test_admin_user, create_test_cert, create_test_flow
|
||||
from authentik.lib.generators import generate_id, generate_key
|
||||
from authentik.policies.models import PolicyBinding
|
||||
from authentik.providers.oauth2.constants import (
|
||||
GRANT_TYPE_CLIENT_CREDENTIALS,
|
||||
|
@ -31,8 +30,6 @@ class TestTokenClientCredentials(OAuthTestCase):
|
|||
self.factory = RequestFactory()
|
||||
self.provider = OAuth2Provider.objects.create(
|
||||
name="test",
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://testserver",
|
||||
signing_key=create_test_cert(),
|
||||
|
|
|
@ -9,7 +9,7 @@ from jwt import decode
|
|||
from authentik.blueprints.tests import apply_blueprint
|
||||
from authentik.core.models import Application, Group
|
||||
from authentik.core.tests.utils import create_test_cert, create_test_flow
|
||||
from authentik.lib.generators import generate_id, generate_key
|
||||
from authentik.lib.generators import generate_id
|
||||
from authentik.policies.models import PolicyBinding
|
||||
from authentik.providers.oauth2.constants import (
|
||||
GRANT_TYPE_CLIENT_CREDENTIALS,
|
||||
|
@ -39,7 +39,7 @@ class TestTokenClientCredentialsJWTSource(OAuthTestCase):
|
|||
slug=generate_id(),
|
||||
provider_type="openidconnect",
|
||||
consumer_key=generate_id(),
|
||||
consumer_secret=generate_key(),
|
||||
consumer_secret=generate_id(),
|
||||
authorization_url="http://foo",
|
||||
access_token_url=f"http://{generate_id()}",
|
||||
profile_url="http://foo",
|
||||
|
@ -52,8 +52,6 @@ class TestTokenClientCredentialsJWTSource(OAuthTestCase):
|
|||
|
||||
self.provider: OAuth2Provider = OAuth2Provider.objects.create(
|
||||
name="test",
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://testserver",
|
||||
signing_key=self.cert,
|
||||
|
|
|
@ -7,7 +7,7 @@ from django.urls import reverse
|
|||
from authentik.blueprints.tests import apply_blueprint
|
||||
from authentik.core.models import Application
|
||||
from authentik.core.tests.utils import create_test_admin_user, create_test_cert, create_test_flow
|
||||
from authentik.lib.generators import generate_code_fixed_length, generate_id, generate_key
|
||||
from authentik.lib.generators import generate_code_fixed_length, generate_id
|
||||
from authentik.providers.oauth2.constants import GRANT_TYPE_DEVICE_CODE
|
||||
from authentik.providers.oauth2.models import DeviceToken, OAuth2Provider, ScopeMapping
|
||||
from authentik.providers.oauth2.tests.utils import OAuthTestCase
|
||||
|
@ -22,8 +22,6 @@ class TestTokenDeviceCode(OAuthTestCase):
|
|||
self.factory = RequestFactory()
|
||||
self.provider = OAuth2Provider.objects.create(
|
||||
name="test",
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="http://testserver",
|
||||
signing_key=create_test_cert(),
|
||||
|
|
|
@ -9,7 +9,7 @@ from authentik.blueprints.tests import apply_blueprint
|
|||
from authentik.core.models import Application
|
||||
from authentik.core.tests.utils import create_test_admin_user, create_test_cert, create_test_flow
|
||||
from authentik.events.models import Event, EventAction
|
||||
from authentik.lib.generators import generate_id, generate_key
|
||||
from authentik.lib.generators import generate_id
|
||||
from authentik.providers.oauth2.models import AccessToken, IDToken, OAuth2Provider, ScopeMapping
|
||||
from authentik.providers.oauth2.tests.utils import OAuthTestCase
|
||||
|
||||
|
@ -23,8 +23,6 @@ class TestUserinfo(OAuthTestCase):
|
|||
self.app = Application.objects.create(name=generate_id(), slug=generate_id())
|
||||
self.provider: OAuth2Provider = OAuth2Provider.objects.create(
|
||||
name=generate_id(),
|
||||
client_id=generate_id(),
|
||||
client_secret=generate_key(),
|
||||
authorization_flow=create_test_flow(),
|
||||
redirect_uris="",
|
||||
signing_key=create_test_cert(),
|
||||
|
|
|
@ -31,7 +31,7 @@ class Migration(migrations.Migration):
|
|||
(
|
||||
"shared_secret",
|
||||
models.TextField(
|
||||
default=authentik.lib.generators.generate_key,
|
||||
default=authentik.lib.generators.generate_id,
|
||||
help_text="Shared secret between clients and server to hash packets.",
|
||||
),
|
||||
),
|
||||
|
|
|
@ -6,7 +6,7 @@ from django.utils.translation import gettext_lazy as _
|
|||
from rest_framework.serializers import Serializer
|
||||
|
||||
from authentik.core.models import Provider
|
||||
from authentik.lib.generators import generate_key
|
||||
from authentik.lib.generators import generate_id
|
||||
from authentik.outposts.models import OutpostModel
|
||||
|
||||
|
||||
|
@ -14,7 +14,7 @@ class RadiusProvider(OutpostModel, Provider):
|
|||
"""Allow applications to authenticate against authentik's users using Radius."""
|
||||
|
||||
shared_secret = models.TextField(
|
||||
default=generate_key,
|
||||
default=generate_id,
|
||||
help_text=_("Shared secret between clients and server to hash packets."),
|
||||
)
|
||||
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
import { RenderFlowOption } from "@goauthentik/admin/flows/utils";
|
||||
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
|
||||
import { first, randomString } from "@goauthentik/common/utils";
|
||||
import { ascii_letters, digits, first, randomString } from "@goauthentik/common/utils";
|
||||
import "@goauthentik/elements/forms/FormGroup";
|
||||
import "@goauthentik/elements/forms/HorizontalFormElement";
|
||||
import { ModelForm } from "@goauthentik/elements/forms/ModelForm";
|
||||
|
@ -203,7 +203,10 @@ export class OAuth2ProviderFormPage extends ModelForm<OAuth2Provider, number> {
|
|||
>
|
||||
<input
|
||||
type="text"
|
||||
value="${first(this.instance?.clientId, randomString(40))}"
|
||||
value="${first(
|
||||
this.instance?.clientId,
|
||||
randomString(40, ascii_letters + digits),
|
||||
)}"
|
||||
class="pf-c-form-control"
|
||||
required
|
||||
/>
|
||||
|
@ -215,7 +218,10 @@ export class OAuth2ProviderFormPage extends ModelForm<OAuth2Provider, number> {
|
|||
>
|
||||
<input
|
||||
type="text"
|
||||
value="${first(this.instance?.clientSecret, randomString(128))}"
|
||||
value="${first(
|
||||
this.instance?.clientSecret,
|
||||
randomString(128, ascii_letters + digits),
|
||||
)}"
|
||||
class="pf-c-form-control"
|
||||
/>
|
||||
</ak-form-element-horizontal>
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
import { RenderFlowOption } from "@goauthentik/admin/flows/utils";
|
||||
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
|
||||
import { first, randomString } from "@goauthentik/common/utils";
|
||||
import { ascii_letters, digits, first, randomString } from "@goauthentik/common/utils";
|
||||
import { rootInterface } from "@goauthentik/elements/Base";
|
||||
import "@goauthentik/elements/forms/FormGroup";
|
||||
import "@goauthentik/elements/forms/HorizontalFormElement";
|
||||
|
@ -98,22 +98,25 @@ export class RadiusProviderFormPage extends ModelForm<RadiusProvider, number> {
|
|||
</ak-search-select>
|
||||
<p class="pf-c-form__helper-text">${t`Flow used for users to authenticate.`}</p>
|
||||
</ak-form-element-horizontal>
|
||||
<ak-form-element-horizontal
|
||||
label=${t`Shared secret`}
|
||||
?required=${true}
|
||||
name="sharedSecret"
|
||||
>
|
||||
<input
|
||||
type="text"
|
||||
value="${first(this.instance?.sharedSecret, randomString(128))}"
|
||||
class="pf-c-form-control"
|
||||
required
|
||||
/>
|
||||
</ak-form-element-horizontal>
|
||||
|
||||
<ak-form-group .expanded=${true}>
|
||||
<span slot="header"> ${t`Protocol settings`} </span>
|
||||
<div slot="body" class="pf-c-form">
|
||||
<ak-form-element-horizontal
|
||||
label=${t`Shared secret`}
|
||||
?required=${true}
|
||||
name="sharedSecret"
|
||||
>
|
||||
<input
|
||||
type="text"
|
||||
value="${first(
|
||||
this.instance?.sharedSecret,
|
||||
randomString(128, ascii_letters + digits),
|
||||
)}"
|
||||
class="pf-c-form-control"
|
||||
required
|
||||
/>
|
||||
</ak-form-element-horizontal>
|
||||
<ak-form-element-horizontal
|
||||
label=${t`Client Networks`}
|
||||
?required=${true}
|
||||
|
|
|
@ -2,7 +2,7 @@ import { RenderFlowOption } from "@goauthentik/admin/flows/utils";
|
|||
import { UserMatchingModeToLabel } from "@goauthentik/admin/sources/oauth/utils";
|
||||
import { DEFAULT_CONFIG, config } from "@goauthentik/common/api/config";
|
||||
import { PlexAPIClient, PlexResource, popupCenterScreen } from "@goauthentik/common/helpers/plex";
|
||||
import { first, randomString } from "@goauthentik/common/utils";
|
||||
import { ascii_letters, digits, first, randomString } from "@goauthentik/common/utils";
|
||||
import { rootInterface } from "@goauthentik/elements/Base";
|
||||
import "@goauthentik/elements/forms/FormGroup";
|
||||
import "@goauthentik/elements/forms/HorizontalFormElement";
|
||||
|
@ -51,7 +51,7 @@ export class PlexSourceForm extends ModelForm<PlexSource, string> {
|
|||
|
||||
get defaultInstance(): PlexSource | undefined {
|
||||
return {
|
||||
clientId: randomString(40),
|
||||
clientId: randomString(40, ascii_letters + digits),
|
||||
} as PlexSource;
|
||||
}
|
||||
|
||||
|
|
|
@ -83,10 +83,23 @@ export function hexEncode(buf: Uint8Array): string {
|
|||
.join("");
|
||||
}
|
||||
|
||||
export function randomString(len: number): string {
|
||||
const arr = new Uint8Array(len / 2);
|
||||
window.crypto.getRandomValues(arr);
|
||||
return hexEncode(arr);
|
||||
// Taken from python's string module
|
||||
export const ascii_lowercase = "abcdefghijklmnopqrstuvwxyz";
|
||||
export const ascii_uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
|
||||
export const ascii_letters = ascii_lowercase + ascii_uppercase;
|
||||
export const digits = "0123456789";
|
||||
export const hexdigits = digits + "abcdef" + "ABCDEF";
|
||||
export const octdigits = "01234567";
|
||||
export const punctuation = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~";
|
||||
|
||||
export function randomString(len: number, charset: string): string {
|
||||
const chars = [];
|
||||
const array = new Uint8Array(len);
|
||||
self.crypto.getRandomValues(array);
|
||||
for (let index = 0; index < len; index++) {
|
||||
chars.push(charset[Math.floor(charset.length * (array[index] / Math.pow(2, 8)))]);
|
||||
}
|
||||
return chars.join("");
|
||||
}
|
||||
|
||||
export function dateTimeLocal(date: Date): string {
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
import { EVENT_REFRESH } from "@goauthentik/common/constants";
|
||||
import { groupBy, randomString } from "@goauthentik/common/utils";
|
||||
import { ascii_letters, digits, groupBy, randomString } from "@goauthentik/common/utils";
|
||||
import { AKElement } from "@goauthentik/elements/Base";
|
||||
import { PreventFormSubmit } from "@goauthentik/elements/forms/Form";
|
||||
|
||||
|
@ -89,7 +89,7 @@ export class SearchSelect<T> extends AKElement {
|
|||
});
|
||||
});
|
||||
this.observer.observe(this);
|
||||
this.dropdownUID = `dropdown-${randomString(10)}`;
|
||||
this.dropdownUID = `dropdown-${randomString(10, ascii_letters + digits)}`;
|
||||
}
|
||||
|
||||
toForm(): unknown {
|
||||
|
|
Reference in a new issue