Skip to content

Commit

Permalink
fix(server): do not get client by id for introspection (#467)
Browse files Browse the repository at this point in the history
As introspection is a Oauth mechanism for resource servers only,
it does not make sense to get an oidc client by ID.
The original OP did not do this and now we make the server behavior similar.
  • Loading branch information
muhlemmer authored Oct 24, 2023
1 parent e5f0dca commit 73a1982
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 25 deletions.
4 changes: 2 additions & 2 deletions pkg/op/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ type Server interface {
// Introspect handles the OAuth 2.0 Token Introspection endpoint.
// https://datatracker.ietf.org/doc/html/rfc7662
// The recommended Response Data type is [oidc.IntrospectionResponse].
Introspect(context.Context, *ClientRequest[oidc.IntrospectionRequest]) (*Response, error)
Introspect(context.Context, *Request[IntrospectionRequest]) (*Response, error)

// UserInfo handles the UserInfo endpoint and returns Claims about the authenticated End-User.
// https://openid.net/specs/openid-connect-core-1_0.html#UserInfo
Expand Down Expand Up @@ -329,7 +329,7 @@ func (UnimplementedServer) DeviceToken(ctx context.Context, r *ClientRequest[oid
return nil, unimplementedGrantError(oidc.GrantTypeDeviceCode)
}

func (UnimplementedServer) Introspect(ctx context.Context, r *ClientRequest[oidc.IntrospectionRequest]) (*Response, error) {
func (UnimplementedServer) Introspect(ctx context.Context, r *Request[IntrospectionRequest]) (*Response, error) {
return nil, unimplementedError(r)
}

Expand Down
37 changes: 25 additions & 12 deletions pkg/op/server_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (s *webServer) createRouter() {
s.endpointRoute(s.endpoints.Authorization, s.authorizeHandler)
s.endpointRoute(s.endpoints.DeviceAuthorization, s.withClient(s.deviceAuthorizationHandler))
s.endpointRoute(s.endpoints.Token, s.tokensHandler)
s.endpointRoute(s.endpoints.Introspection, s.withClient(s.introspectionHandler))
s.endpointRoute(s.endpoints.Introspection, s.introspectionHandler)
s.endpointRoute(s.endpoints.Userinfo, s.userInfoHandler)
s.endpointRoute(s.endpoints.Revocation, s.withClient(s.revocationHandler))
s.endpointRoute(s.endpoints.EndSession, s.endSessionHandler)
Expand Down Expand Up @@ -136,7 +136,21 @@ func (s *webServer) withClient(handler clientHandler) http.HandlerFunc {
}

func (s *webServer) verifyRequestClient(r *http.Request) (_ Client, err error) {
if err = r.ParseForm(); err != nil {
cc, err := s.parseClientCredentials(r)
if err != nil {
return nil, err
}
return s.server.VerifyClient(r.Context(), &Request[ClientCredentials]{
Method: r.Method,
URL: r.URL,
Header: r.Header,
Form: r.Form,
Data: cc,
})
}

func (s *webServer) parseClientCredentials(r *http.Request) (_ *ClientCredentials, err error) {
if err := r.ParseForm(); err != nil {
return nil, oidc.ErrInvalidRequest().WithDescription("error parsing form").WithParent(err)
}
cc := new(ClientCredentials)
Expand All @@ -160,13 +174,7 @@ func (s *webServer) verifyRequestClient(r *http.Request) (_ Client, err error) {
if cc.ClientAssertion != "" && cc.ClientAssertionType != oidc.ClientAssertionTypeJWTAssertion {
return nil, oidc.ErrInvalidRequest().WithDescription("invalid client_assertion_type %s", cc.ClientAssertionType)
}
return s.server.VerifyClient(r.Context(), &Request[ClientCredentials]{
Method: r.Method,
URL: r.URL,
Header: r.Header,
Form: r.Form,
Data: cc,
})
return cc, nil
}

func (s *webServer) authorizeHandler(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -378,8 +386,13 @@ func (s *webServer) deviceTokenHandler(w http.ResponseWriter, r *http.Request, c
resp.writeOut(w)
}

func (s *webServer) introspectionHandler(w http.ResponseWriter, r *http.Request, client Client) {
if client.AuthMethod() == oidc.AuthMethodNone {
func (s *webServer) introspectionHandler(w http.ResponseWriter, r *http.Request) {
cc, err := s.parseClientCredentials(r)
if err != nil {
WriteError(w, r, err, s.getLogger(r.Context()))
return
}
if cc.ClientSecret == "" && cc.ClientAssertion == "" {
WriteError(w, r, oidc.ErrInvalidClient().WithDescription("client must be authenticated"), s.getLogger(r.Context()))
return
}
Expand All @@ -392,7 +405,7 @@ func (s *webServer) introspectionHandler(w http.ResponseWriter, r *http.Request,
WriteError(w, r, oidc.ErrInvalidRequest().WithDescription("token missing"), s.getLogger(r.Context()))
return
}
resp, err := s.server.Introspect(r.Context(), newClientRequest(r, request, client))
resp, err := s.server.Introspect(r.Context(), newRequest(r, &IntrospectionRequest{cc, request}))
if err != nil {
WriteError(w, r, err, s.getLogger(r.Context()))
return
Expand Down
13 changes: 4 additions & 9 deletions pkg/op/server_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1001,14 +1001,12 @@ func Test_webServer_introspectionHandler(t *testing.T) {
tests := []struct {
name string
decoder httphelper.Decoder
client Client
r *http.Request
want webServerResult
}{
{
name: "decoder error",
decoder: schema.NewDecoder(),
client: newClient(clientTypeUserAgent),
r: httptest.NewRequest(http.MethodPost, "/", strings.NewReader("foo=bar")),
want: webServerResult{
wantStatus: http.StatusBadRequest,
Expand All @@ -1018,8 +1016,7 @@ func Test_webServer_introspectionHandler(t *testing.T) {
{
name: "public client",
decoder: testDecoder,
client: newClient(clientTypeNative),
r: httptest.NewRequest(http.MethodPost, "/", strings.NewReader("foo=bar")),
r: httptest.NewRequest(http.MethodPost, "/", strings.NewReader("client_id=123")),
want: webServerResult{
wantStatus: http.StatusBadRequest,
wantBody: `{"error":"invalid_client", "error_description":"client must be authenticated"}`,
Expand All @@ -1028,8 +1025,7 @@ func Test_webServer_introspectionHandler(t *testing.T) {
{
name: "token missing",
decoder: testDecoder,
client: newClient(clientTypeWeb),
r: httptest.NewRequest(http.MethodPost, "/", strings.NewReader("foo=bar")),
r: httptest.NewRequest(http.MethodPost, "/", strings.NewReader("client_id=123&client_secret=SECRET")),
want: webServerResult{
wantStatus: http.StatusBadRequest,
wantBody: `{"error":"invalid_request", "error_description":"token missing"}`,
Expand All @@ -1038,8 +1034,7 @@ func Test_webServer_introspectionHandler(t *testing.T) {
{
name: "unimplemented Introspect called",
decoder: testDecoder,
client: newClient(clientTypeWeb),
r: httptest.NewRequest(http.MethodPost, "/", strings.NewReader("token=xxx")),
r: httptest.NewRequest(http.MethodPost, "/", strings.NewReader("client_id=123&client_secret=SECRET&token=xxx")),
want: webServerResult{
wantStatus: UnimplementedStatusCode,
wantBody: `{"error":"server_error", "error_description":"/ not implemented on this server"}`,
Expand All @@ -1053,7 +1048,7 @@ func Test_webServer_introspectionHandler(t *testing.T) {
decoder: tt.decoder,
logger: slog.Default(),
}
runWebServerClientTest(t, s.introspectionHandler, tt.r, tt.client, tt.want)
runWebServerTest(t, s.introspectionHandler, tt.r, tt.want)
})
}
}
Expand Down
21 changes: 19 additions & 2 deletions pkg/op/server_legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,13 +315,30 @@ func (s *LegacyServer) DeviceToken(ctx context.Context, r *ClientRequest[oidc.De
return NewResponse(resp), nil
}

func (s *LegacyServer) Introspect(ctx context.Context, r *ClientRequest[oidc.IntrospectionRequest]) (*Response, error) {
func (s *LegacyServer) authenticateResourceClient(ctx context.Context, cc *ClientCredentials) (string, error) {
if cc.ClientAssertion != "" {
if jp, ok := s.provider.(ClientJWTProfile); ok {
return ClientJWTAuth(ctx, oidc.ClientAssertionParams{ClientAssertion: cc.ClientAssertion}, jp)
}
return "", oidc.ErrInvalidClient().WithDescription("client_assertion not supported")
}
if err := s.provider.Storage().AuthorizeClientIDSecret(ctx, cc.ClientID, cc.ClientSecret); err != nil {
return "", oidc.ErrUnauthorizedClient().WithParent(err)
}
return cc.ClientID, nil
}

func (s *LegacyServer) Introspect(ctx context.Context, r *Request[IntrospectionRequest]) (*Response, error) {
clientID, err := s.authenticateResourceClient(ctx, r.Data.ClientCredentials)
if err != nil {
return nil, err
}
response := new(oidc.IntrospectionResponse)
tokenID, subject, ok := getTokenIDAndSubject(ctx, s.provider, r.Data.Token)
if !ok {
return NewResponse(response), nil
}
err := s.provider.Storage().SetIntrospectionFromToken(ctx, response, tokenID, subject, r.Client.GetID())
err = s.provider.Storage().SetIntrospectionFromToken(ctx, response, tokenID, subject, clientID)
if err != nil {
return NewResponse(response), nil
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/op/token_intospection.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,8 @@ func ParseTokenIntrospectionRequest(r *http.Request, introspector Introspector)

return req.Token, clientID, nil
}

type IntrospectionRequest struct {
*ClientCredentials
*oidc.IntrospectionRequest
}

0 comments on commit 73a1982

Please sign in to comment.