Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/modules/auth/openid/openid.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,8 @@ func exchangeOidcTokens(cb *Callback, providerKey string) (*Provider, *oauth2.To
// Parse the access & ID token
oauth2Token, err := provider.Oauth2Config.Exchange(context.Background(), cb.Code)
if err != nil {
log.Debugf("Token exchange failed for provider %s using token_endpoint_auth_method %s", provider.Key, authStyleName(provider.Oauth2Config.Endpoint.AuthStyle))

var rerr *oauth2.RetrieveError
if errors.As(err, &rerr) {

Expand Down
50 changes: 48 additions & 2 deletions pkg/modules/auth/openid/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package openid
import (
"errors"
"fmt"
"slices"
"strconv"

"code.vikunja.io/api/pkg/config"
Expand Down Expand Up @@ -356,11 +357,14 @@ func getProviderFromMap(pi map[string]interface{}, key string) (provider *Provid
return
}

// Discovery returns the OAuth2 endpoints, but not the auth style.
endpoint := provider.openIDProvider.Endpoint()
endpoint.AuthStyle = provider.discoveredTokenEndpointAuthStyle()

provider.Oauth2Config = &oauth2.Config{
ClientID: provider.ClientID,
ClientSecret: provider.ClientSecret,
// Discovery returns the OAuth2 endpoints.
Endpoint: provider.openIDProvider.Endpoint(),
Endpoint: endpoint,

// "openid" is a required scope for OpenID Connect flows.
Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
Expand All @@ -373,6 +377,48 @@ func getProviderFromMap(pi map[string]interface{}, key string) (provider *Provid
return
}

const (
authMethodBasic = "client_secret_basic"
authMethodPost = "client_secret_post"
)

func authStyleName(style oauth2.AuthStyle) string {
switch style {
case oauth2.AuthStyleInHeader:
return authMethodBasic
case oauth2.AuthStyleInParams:
return authMethodPost
case oauth2.AuthStyleAutoDetect:
return "auto"
default:
return "auto"
}
}

// Fallback preserves autodetection for OIDC providers that omit the discovery field.
Comment thread
tink-bot marked this conversation as resolved.
func (p *Provider) discoveredTokenEndpointAuthStyle() oauth2.AuthStyle {
if p.openIDProvider == nil {
return oauth2.AuthStyleAutoDetect
}

var meta struct {
TokenEndpointAuthMethodsSupported []string `json:"token_endpoint_auth_methods_supported"`
}
if err := p.openIDProvider.Claims(&meta); err != nil {
log.Debugf("Could not read token_endpoint_auth_methods_supported for provider %s: %v", p.Key, err)
return oauth2.AuthStyleAutoDetect
}

switch {
case slices.Contains(meta.TokenEndpointAuthMethodsSupported, authMethodBasic):
return oauth2.AuthStyleInHeader
case slices.Contains(meta.TokenEndpointAuthMethodsSupported, authMethodPost):
return oauth2.AuthStyleInParams
default:
return oauth2.AuthStyleAutoDetect
}
}

// CleanupSavedOpenIDProviders removes all cached provider state so the next
// GetAllProviders call rebuilds it from config. The per-provider entries must
// be removed too: GetProvider resolves them before the provider list, so a
Expand Down
144 changes: 144 additions & 0 deletions pkg/modules/auth/openid/providers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package openid

import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
Expand All @@ -27,6 +28,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/oauth2"
)

func TestGetAllProvidersTypeSafety(t *testing.T) {
Expand Down Expand Up @@ -94,6 +96,11 @@ func TestGetAllProvidersTypeSafety(t *testing.T) {
// newMockOIDCServer creates a test HTTP server that serves a valid OIDC discovery document.
// The issuer in the discovery document matches the server's URL.
func newMockOIDCServer() *httptest.Server {
return newMockOIDCServerWithAuthMethods(nil)
}

// A nil slice omits token_endpoint_auth_methods_supported.
func newMockOIDCServerWithAuthMethods(authMethods []string, tokenHandlers ...http.HandlerFunc) *httptest.Server {
var server *httptest.Server
mux := http.NewServeMux()
mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) {
Expand All @@ -103,9 +110,19 @@ func newMockOIDCServer() *httptest.Server {
"token_endpoint": server.URL + "/token",
"jwks_uri": server.URL + "/jwks",
}
if authMethods != nil {
discovery["token_endpoint_auth_methods_supported"] = authMethods
}
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(discovery)
})
tokenHandler := func(w http.ResponseWriter, _ *http.Request) {
http.Error(w, "not implemented", http.StatusNotImplemented)
}
if len(tokenHandlers) > 0 {
tokenHandler = tokenHandlers[0]
}
mux.HandleFunc("/token", tokenHandler)
server = httptest.NewServer(mux)
return server
}
Expand Down Expand Up @@ -344,3 +361,130 @@ func TestFailedDiscoverySkippedInIssuerCheck(t *testing.T) {
assert.Len(t, providers, 1)
assert.Equal(t, "Valid Provider", providers[0].Name)
}

func TestTokenEndpointAuthStyleFromDiscovery(t *testing.T) {
cases := []struct {
name string
authMethods []string
want oauth2.AuthStyle
}{
{
name: "basic only",
authMethods: []string{"client_secret_basic"},
want: oauth2.AuthStyleInHeader,
},
{
name: "post only",
authMethods: []string{"client_secret_post"},
want: oauth2.AuthStyleInParams,
},
{
name: "both advertised prefers basic",
authMethods: []string{"client_secret_post", "client_secret_basic"},
want: oauth2.AuthStyleInHeader,
},
{
name: "neither advertised falls back to autodetect",
authMethods: []string{"private_key_jwt", "none"},
want: oauth2.AuthStyleAutoDetect,
},
{
name: "empty list falls back to autodetect",
authMethods: []string{},
want: oauth2.AuthStyleAutoDetect,
},
{
name: "field missing falls back to autodetect",
authMethods: nil,
want: oauth2.AuthStyleAutoDetect,
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
defer CleanupSavedOpenIDProviders()

server := newMockOIDCServerWithAuthMethods(tc.authMethods)
defer server.Close()

provider, err := getProviderFromMap(map[string]interface{}{
"name": "Test Provider",
"authurl": server.URL,
"clientid": "client1",
"clientsecret": "secret1",
}, "test")
require.NoError(t, err)
require.NotNil(t, provider)

assert.Equal(t, tc.want, provider.Oauth2Config.Endpoint.AuthStyle)
})
}

t.Run("basic only exchanges once with header credentials", func(t *testing.T) {
Comment thread
tink-bot marked this conversation as resolved.
requestCount := 0
var authorization, clientSecret string
var basicParseFormErr error
server := newMockOIDCServerWithAuthMethods([]string{authMethodBasic}, func(w http.ResponseWriter, r *http.Request) {
requestCount++
authorization = r.Header.Get("Authorization")
basicParseFormErr = r.ParseForm()
clientSecret = r.Form.Get("client_secret")
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusUnauthorized)
_, _ = w.Write([]byte(`{"error":"invalid_client","error_description":"distinctive wrong secret"}`))
})
defer server.Close()

provider, err := getProviderFromMap(map[string]interface{}{
"name": "Test Provider",
"authurl": server.URL,
"clientid": "client1",
"clientsecret": "wrong-secret",
}, "test")
require.NoError(t, err)

_, err = provider.Oauth2Config.Exchange(context.Background(), "authorization-code")
require.NoError(t, basicParseFormErr)
require.Error(t, err)
var retrieveErr *oauth2.RetrieveError
require.ErrorAs(t, err, &retrieveErr)
assert.Equal(t, "invalid_client", retrieveErr.ErrorCode)
assert.Equal(t, "distinctive wrong secret", retrieveErr.ErrorDescription)
assert.Equal(t, 1, requestCount)
assert.Equal(t, "Basic Y2xpZW50MTp3cm9uZy1zZWNyZXQ=", authorization)
assert.Empty(t, clientSecret)
})

t.Run("post only exchanges once with form credentials", func(t *testing.T) {
requestCount := 0
var authorization, clientID, clientSecret string
var postParseFormErr error
server := newMockOIDCServerWithAuthMethods([]string{authMethodPost}, func(w http.ResponseWriter, r *http.Request) {
requestCount++
authorization = r.Header.Get("Authorization")
postParseFormErr = r.ParseForm()
clientID = r.Form.Get("client_id")
clientSecret = r.Form.Get("client_secret")
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusUnauthorized)
_, _ = w.Write([]byte(`{"error":"invalid_client"}`))
})
defer server.Close()

provider, err := getProviderFromMap(map[string]interface{}{
"name": "Test Provider",
"authurl": server.URL,
"clientid": "client1",
"clientsecret": "secret1",
}, "test")
require.NoError(t, err)

_, err = provider.Oauth2Config.Exchange(context.Background(), "authorization-code")
require.NoError(t, postParseFormErr)
require.Error(t, err)
assert.Equal(t, 1, requestCount)
assert.Empty(t, authorization)
assert.Equal(t, "client1", clientID)
assert.Equal(t, "secret1", clientSecret)
})
}
Loading