Skip to content

feat(oidc): allow disabling client_secret on external flow - #1028

Closed
gekmihesg wants to merge 1 commit into
gotify:masterfrom
gekmihesg:master
Closed

feat(oidc): allow disabling client_secret on external flow#1028
gekmihesg wants to merge 1 commit into
gotify:masterfrom
gekmihesg:master

Conversation

@gekmihesg

Copy link
Copy Markdown

Add GOTIFY_OIDC_EXTERNALSECRET to allow omitting the client_secret from OIDC token exchange for the external (mobile app) flow.

This is required to get OIDC to work with Microsoft Entra that rejects requests containing a client_secret for mobile and desktop applications as they are marked as public client (gotify://oidc/callback).

The error I got when trying to use OIDC from the mobile app: errors="Error #01: token exchange failed: oauth2: "invalid_client" "AADSTS700025: Client is public so neither 'client_assertion' nor 'client_secret' should be presented.
In the browser, it worked fine. With this patch, OIDC works as expected for the mobile app as well.

@gekmihesg
gekmihesg requested a review from a team as a code owner August 12, 2026 08:33
Add GOTIFY_OIDC_EXTERNALSECRET to allow omitting the client_secret from
OIDC token exchange for the external (mobile app) flow.

This is required to get OIDC to work with Microsoft Entra that rejects
requests containing a client_secret for mobile and desktop applications
as they are marked as public client (gotify://oidc/callback).
Comment thread api/oidc.go
context.Background(),
conf.OIDC.Issuer,
conf.OIDC.ClientID,
"", // no secret, PKCE only

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you use a public client for the web ui too? Keep GOTIFY_OIDC_CLIENTSECRET unset or set to an empty string. Then we don't need this workaround for Entra.

@gekmihesg gekmihesg Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, that works as well. The change took some iterations and had more workarounds added but they all proved to be unnecessary and this was the only part left. So, nevermind then, feel free to close this PR.

@jmattheis jmattheis closed this Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants