diff --git a/internal/api/oidc/client_integration_test.go b/internal/api/oidc/client_integration_test.go index 1593fbb8e9..a20e388dca 100644 --- a/internal/api/oidc/client_integration_test.go +++ b/internal/api/oidc/client_integration_test.go @@ -61,46 +61,116 @@ func TestServer_Introspect(t *testing.T) { require.NoError(t, err) app, err := Tester.CreateOIDCNativeClient(CTX, redirectURI, logoutRedirectURI, project.GetId(), false) require.NoError(t, err) - api, err := Tester.CreateAPIClient(CTX, project.GetId()) - require.NoError(t, err) - keyResp, err := Tester.Client.Mgmt.AddAppKey(CTX, &management.AddAppKeyRequest{ - ProjectId: project.GetId(), - AppId: api.GetAppId(), - Type: authn.KeyType_KEY_TYPE_JSON, - ExpirationDate: nil, - }) - require.NoError(t, err) - resourceServer, err := Tester.CreateResourceServer(CTX, keyResp.GetKeyDetails()) - require.NoError(t, err) - scope := []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeEmail, oidc.ScopeOfflineAccess, oidc_api.ScopeResourceOwner} - authRequestID := createAuthRequest(t, app.GetClientId(), redirectURI, scope...) - sessionID, sessionToken, startTime, changeTime := Tester.CreateVerifiedWebAuthNSession(t, CTXLOGIN, User.GetUserId()) - linkResp, err := Tester.Client.OIDCv2.CreateCallback(CTXLOGIN, &oidc_pb.CreateCallbackRequest{ - AuthRequestId: authRequestID, - CallbackKind: &oidc_pb.CreateCallbackRequest_Session{ - Session: &oidc_pb.Session{ - SessionId: sessionID, - SessionToken: sessionToken, + wantAudience := []string{app.GetClientId(), project.GetId()} + + tests := []struct { + name string + api func(*testing.T) (apiID string, resourceServer rs.ResourceServer) + wantErr bool + }{ + { + name: "client assertion", + api: func(t *testing.T) (string, rs.ResourceServer) { + api, err := Tester.CreateAPIClientJWT(CTX, project.GetId()) + require.NoError(t, err) + keyResp, err := Tester.Client.Mgmt.AddAppKey(CTX, &management.AddAppKeyRequest{ + ProjectId: project.GetId(), + AppId: api.GetAppId(), + Type: authn.KeyType_KEY_TYPE_JSON, + ExpirationDate: nil, + }) + require.NoError(t, err) + resourceServer, err := Tester.CreateResourceServerJWTProfile(CTX, keyResp.GetKeyDetails()) + require.NoError(t, err) + return api.GetClientId(), resourceServer }, }, - }) - require.NoError(t, err) + { + name: "client credentials", + api: func(t *testing.T) (string, rs.ResourceServer) { + api, err := Tester.CreateAPIClientBasic(CTX, project.GetId()) + require.NoError(t, err) + resourceServer, err := Tester.CreateResourceServerClientCredentials(CTX, api.GetClientId(), api.GetClientSecret()) + require.NoError(t, err) + return api.GetClientId(), resourceServer + }, + }, + { + name: "client invalid id, error", + api: func(t *testing.T) (string, rs.ResourceServer) { + api, err := Tester.CreateAPIClientBasic(CTX, project.GetId()) + require.NoError(t, err) + resourceServer, err := Tester.CreateResourceServerClientCredentials(CTX, "xxxxx", api.GetClientSecret()) + require.NoError(t, err) + return api.GetClientId(), resourceServer + }, + wantErr: true, + }, + { + name: "client invalid secret, error", + api: func(t *testing.T) (string, rs.ResourceServer) { + api, err := Tester.CreateAPIClientBasic(CTX, project.GetId()) + require.NoError(t, err) + resourceServer, err := Tester.CreateResourceServerClientCredentials(CTX, api.GetClientId(), "xxxxx") + require.NoError(t, err) + return api.GetClientId(), resourceServer + }, + wantErr: true, + }, + { + name: "client credentials on jwt client, error", + api: func(t *testing.T) (string, rs.ResourceServer) { + api, err := Tester.CreateAPIClientJWT(CTX, project.GetId()) + require.NoError(t, err) + resourceServer, err := Tester.CreateResourceServerClientCredentials(CTX, api.GetClientId(), "xxxxx") + require.NoError(t, err) + return api.GetClientId(), resourceServer + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + apiID, resourceServer := tt.api(t) + // wantAudience grows for every API we add to the project. + wantAudience = append(wantAudience, apiID) - // code exchange - code := assertCodeResponse(t, linkResp.GetCallbackUrl()) - tokens, err := exchangeTokens(t, app.GetClientId(), code) - require.NoError(t, err) - assertTokens(t, tokens, true) - assertIDTokenClaims(t, tokens.IDTokenClaims, armPasskey, startTime, changeTime) + scope := []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeEmail, oidc.ScopeOfflineAccess, oidc_api.ScopeResourceOwner} + authRequestID := createAuthRequest(t, app.GetClientId(), redirectURI, scope...) + sessionID, sessionToken, startTime, changeTime := Tester.CreateVerifiedWebAuthNSession(t, CTXLOGIN, User.GetUserId()) + linkResp, err := Tester.Client.OIDCv2.CreateCallback(CTXLOGIN, &oidc_pb.CreateCallbackRequest{ + AuthRequestId: authRequestID, + CallbackKind: &oidc_pb.CreateCallbackRequest_Session{ + Session: &oidc_pb.Session{ + SessionId: sessionID, + SessionToken: sessionToken, + }, + }, + }) + require.NoError(t, err) - // test actual introspection - introspection, err := rs.Introspect[*oidc.IntrospectionResponse](context.Background(), resourceServer, tokens.AccessToken) - require.NoError(t, err) - assertIntrospection(t, introspection, - Tester.OIDCIssuer(), app.GetClientId(), - scope, []string{app.GetClientId(), api.GetClientId(), project.GetId()}, - tokens.Expiry, tokens.Expiry.Add(-12*time.Hour)) + // code exchange + code := assertCodeResponse(t, linkResp.GetCallbackUrl()) + tokens, err := exchangeTokens(t, app.GetClientId(), code) + require.NoError(t, err) + assertTokens(t, tokens, true) + assertIDTokenClaims(t, tokens.IDTokenClaims, armPasskey, startTime, changeTime) + + // test actual introspection + introspection, err := rs.Introspect[*oidc.IntrospectionResponse](context.Background(), resourceServer, tokens.AccessToken) + if tt.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assertIntrospection(t, introspection, + Tester.OIDCIssuer(), app.GetClientId(), + scope, wantAudience, + tokens.Expiry, tokens.Expiry.Add(-12*time.Hour)) + }) + } } func assertUserinfo(t *testing.T, userinfo *oidc.UserInfo) { diff --git a/internal/api/oidc/introspect.go b/internal/api/oidc/introspect.go index 90cda8de7a..8c73755199 100644 --- a/internal/api/oidc/introspect.go +++ b/internal/api/oidc/introspect.go @@ -72,7 +72,7 @@ func (s *Server) Introspect(ctx context.Context, r *op.Request[op.IntrospectionR return nil, err } - // remaining errors shoudn't be returned to the client, + // remaining errors shouldn't be returned to the client, // so we catch errors here, log them and return the response // with active: false defer func() { @@ -122,6 +122,8 @@ type introspectionClientResult struct { err error } +var errNoClientSecret = errors.New("client has no configured secret") + func (s *Server) introspectionClientAuth(ctx context.Context, cc *op.ClientCredentials, rc chan<- *introspectionClientResult) { ctx, span := tracing.NewSpan(ctx) @@ -136,13 +138,16 @@ func (s *Server) introspectionClientAuth(ctx context.Context, cc *op.ClientCrede if _, err := op.VerifyJWTAssertion(ctx, cc.ClientAssertion, verifier); err != nil { return "", "", oidc.ErrUnauthorizedClient().WithParent(err) } - } else { + return client.ClientID, client.ProjectID, nil + + } + if client.ClientSecret != nil { if err := crypto.CompareHash(client.ClientSecret, []byte(cc.ClientSecret), s.hashAlg); err != nil { return "", "", oidc.ErrUnauthorizedClient().WithParent(err) } + return client.ClientID, client.ProjectID, nil } - - return client.ClientID, client.ProjectID, nil + return "", "", oidc.ErrUnauthorizedClient().WithParent(errNoClientSecret) }() span.EndWithError(err) diff --git a/internal/integration/oidc.go b/internal/integration/oidc.go index e49b4e9c7e..16c4c90ae5 100644 --- a/internal/integration/oidc.go +++ b/internal/integration/oidc.go @@ -119,7 +119,7 @@ func (s *Tester) CreateProject(ctx context.Context) (*management.AddProjectRespo }) } -func (s *Tester) CreateAPIClient(ctx context.Context, projectID string) (*management.AddAPIAppResponse, error) { +func (s *Tester) CreateAPIClientJWT(ctx context.Context, projectID string) (*management.AddAPIAppResponse, error) { return s.Client.Mgmt.AddAPIApp(ctx, &management.AddAPIAppRequest{ ProjectId: projectID, Name: fmt.Sprintf("api-%d", time.Now().UnixNano()), @@ -127,6 +127,14 @@ func (s *Tester) CreateAPIClient(ctx context.Context, projectID string) (*manage }) } +func (s *Tester) CreateAPIClientBasic(ctx context.Context, projectID string) (*management.AddAPIAppResponse, error) { + return s.Client.Mgmt.AddAPIApp(ctx, &management.AddAPIAppRequest{ + ProjectId: projectID, + Name: fmt.Sprintf("api-%d", time.Now().UnixNano()), + AuthMethodType: app.APIAuthMethodType_API_AUTH_METHOD_TYPE_BASIC, + }) +} + const CodeVerifier = "codeVerifier" func (s *Tester) CreateOIDCAuthRequest(ctx context.Context, clientID, loginClient, redirectURI string, scope ...string) (authRequestID string, err error) { @@ -207,7 +215,7 @@ func (c *loginRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) return c.RoundTripper.RoundTrip(req) } -func (s *Tester) CreateResourceServer(ctx context.Context, keyFileData []byte) (rs.ResourceServer, error) { +func (s *Tester) CreateResourceServerJWTProfile(ctx context.Context, keyFileData []byte) (rs.ResourceServer, error) { keyFile, err := client.ConfigFromKeyFileData(keyFileData) if err != nil { return nil, err @@ -215,6 +223,10 @@ func (s *Tester) CreateResourceServer(ctx context.Context, keyFileData []byte) ( return rs.NewResourceServerJWTProfile(ctx, s.OIDCIssuer(), keyFile.ClientID, keyFile.KeyID, []byte(keyFile.Key)) } +func (s *Tester) CreateResourceServerClientCredentials(ctx context.Context, clientID, clientSecret string) (rs.ResourceServer, error) { + return rs.NewResourceServerClientCredentials(ctx, s.OIDCIssuer(), clientID, clientSecret) +} + func GetRequest(url string, headers map[string]string) (*http.Request, error) { req, err := http.NewRequest(http.MethodGet, url, nil) if err != nil {