Repository navigation
Conversation
✅ Deploy Preview for authentik-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
9a22fc8 to
ee0cb37
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dominic/rotation-providers #26100 +/- ##
==============================================================
+ Coverage 92.41% 92.53% +0.12%
==============================================================
Files 1207 1208 +1
Lines 81843 81954 +111
Branches 4489 4489
==============================================================
+ Hits 75634 75838 +204
+ Misses 6143 6050 -93
Partials 66 66
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
|
authentik PR Installation instructions Instructions for docker-composeAdd the following block to your AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-eed79ff0cb6d09898235bd83460aa24c6acf078e
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)sAfterwards, run the upgrade commands from the latest release notes. Instructions for KubernetesAdd the following block to your authentik:
outposts:
container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
global:
image:
repository: ghcr.io/goauthentik/dev-server
tag: gh-eed79ff0cb6d09898235bd83460aa24c6acf078eAfterwards, run the upgrade commands from the latest release notes. |
ee0cb37 to
c7613e4
Compare
c7613e4 to
835a51e
Compare
835a51e to
8ab59ee
Compare
8ab59ee to
03f1249
Compare
03f1249 to
cf7295a
Compare
…s-scim # Conflicts: # packages/client-ts/src/models/PatchedSCIMProviderRequest.ts # packages/client-ts/src/models/RadiusProvider.ts # packages/client-ts/src/models/SCIMProvider.ts # packages/client-ts/src/models/SCIMProviderRequest.ts
7e3015c to
9d951e8
Compare
…s-scim # Conflicts: # web/src/admin/providers/radius/RadiusProviderFormForm.ts # web/src/admin/providers/scim/SCIMProviderFormForm.ts
…quired RADIUS providers generate their shared secret with their own generator again and always reference a secret, like OAuth2 providers, so clearing the secret of an existing provider fails instead of replacing it. SCIM keeps its legacy columns where they were, migrates both credentials as text, and the form shows field errors and requires a token or password when creating a provider again.
melizeche
left a comment
melizeche
left a comment
There was a problem hiding this comment.
Looks good overall, same pattern as OAuth2. Left three comments: RADIUS providers sharing a secret, SCIM sending its token to a changed URL, and the rotated RADIUS secret length
| # Shared secret is not a write-only field, as | ||
| # an admin might have to view it | ||
| "shared_secret", | ||
| "shared_secret_ref", |
There was a problem hiding this comment.
The RADIUS outpost finds the provider by its shared secret:
authentik/internal/outpost/radius/handler.go
Lines 68 to 76 in 94fd54e
and the first match wins. If two providers on the same outpost share a secret, requests can use the wrong provider’s flow and application policies. The picker makes this easy to configure now. Could we prevent that combination, or adjust the lookup so sharing a secret doesn’t make provider selection ambiguous?
There was a problem hiding this comment.
Fixed in 836aa1b. The API and blueprints reject a shared secret that another RADIUS provider already uses. The docs say RADIUS providers can't share a secret.
|
|
||
| def __call__(self, request: Request) -> Request: | ||
| request.headers["Authorization"] = f"Bearer {self.provider.token}" | ||
| token = self.provider.token_ref.secret_value if self.provider.token_ref else "" |
There was a problem hiding this comment.
A user with change_scimprovider but without view_secret_value can still change url to a server they control and receive the existing token on the next sync. This predates the PR, but should changing the destination require permission to view the attached secret? Email, webhook and LDAP configurations probably have similar considerations wherever an editable destination receives stored credentials. If that’s out of scope, documenting the limitation would help
There was a problem hiding this comment.
Out of scope here, since it predates the stack and applies to every object that sends a stored credential to a destination you can edit. I documented it in df45d66 as a warning on the permissions page: changing where an object sends its credential sends the credential there, with SCIM URLs and LDAP and email servers as examples. Only assign change permission on those objects to roles that may know their credentials.
There was a problem hiding this comment.
Extended the docs warning in 361075c to the other objects that send a stored credential to a destination you can edit: LDAP sources, email and email authenticator setup stages, OAuth source token URLs, generic SMS provider URLs, and Fleet connector URLs.
| class RadiusProvider(GeneratedSecretsMixin, OutpostModel, Provider): | ||
| """Allow applications to authenticate against authentik's users using Radius.""" | ||
|
|
||
| generated_secrets = {"shared_secret_ref": ("shared secret", generate_id)} |
There was a problem hiding this comment.
Rotation increases this to at least default_token_length (60 by default). Some RADIUS clients have lower limits; e.g: Extreme ERS 3600 documents a maximum of 16 characters. So rotating a manually configured short secret can produce a value the client can’t accept, even when the admin updates both sides. Could rotation preserve the current length, or let the admin choose a compatible length?
There was a problem hiding this comment.
Fixed in 19a1ec2. Rotation now keeps the current length, so a 16-character secret stays 16 characters. The default token length only applies to new generated secrets. To change the length, replace the value.
Details
Part of the managed secrets stack, see #25415.
What does this PR change?
RADIUS providers store their shared secret, and SCIM providers their token and Basic authentication password, as secrets. Like OAuth2 providers, RADIUS providers generate their shared secret (40 characters) when none is selected and always reference one.
Why is this change needed?
Each credential becomes a secret with its own permissions, rotation and audit trail. See #25415.
How was this tested?
Added tests for generated shared secrets, rejecting a cleared reference, and SCIM token and Basic authentication. Checked that outposts receive a rotated shared secret.
Linked issues
Checklist
make all)make docs)