Add some more tests & refactor some code

Signed-off-by: Romain Caire <[email protected]>

server/introspectionhandler.go

@@ -52,6 +52,8 @@ func (s *Server) getTokenFromRequest(r *http.Request) (string, string, error) {
 		return "", "", newIntrospectBadRequestError("Unable to parse HTTP body, make sure to send a properly formatted form request body.")
 	} else if r.PostForm == nil || len(r.PostForm) == 0 {
 		return "", "", newIntrospectBadRequestError("The POST body can not be empty.")
+	} else if !r.PostForm.Has("token") {
+		return "", "", newIntrospectBadRequestError("The POST body doesn't contain 'token' parameter.")
 	}
 
 	return r.PostForm.Get("token"), r.PostForm.Get("token_type_hint"), nil
@@ -75,18 +77,23 @@ func (s *Server) introspectRefreshToken(_ context.Context, token string) (*Intro
 			return nil, newIntrospectInactiveTokenError()
 		}
 
+		s.logger.Errorf("failed to get refresh token: %v", err)
 		return nil, newIntrospectInternalServerError()
 	}
 
-	return &Introspection{
-		Active:     true,
-		JwtTokenID: rCtx.storageToken.ID,
+	subjectString, sErr := s.genSubject(rCtx.storageToken.Claims.UserID, rCtx.storageToken.ConnectorID)
+	if sErr != nil {
+		s.logger.Errorf("failed to marshal offline session ID: %v", err)
+		return nil, newIntrospectInternalServerError()
+	}
 
+	return &Introspection{
+		Active:    true,
 		ClientID:  rCtx.storageToken.ClientID,
 		IssuedAt:  rCtx.storageToken.CreatedAt.Unix(),
 		NotBefore: rCtx.storageToken.CreatedAt.Unix(),
 		Expiry:    rCtx.storageToken.CreatedAt.Add(s.refreshTokenPolicy.absoluteLifetime).Unix(),
-		Subject:   rCtx.storageToken.Claims.UserID,
+		Subject:   subjectString,
 		Username:  rCtx.storageToken.Claims.PreferredUsername,
 		Audience:  s.getAudience(rCtx.storageToken.ClientID, rCtx.scopes),
 		Issuer:    s.issuerURL.String(),
@@ -126,9 +133,7 @@ func (s *Server) introspectAccessToken(ctx context.Context, token string) (*Intr
 	}
 
 	return &Introspection{
-		Active:     true,
-		JwtTokenID: idToken.AccessTokenHash,
-
+		Active:    true,
 		ClientID:  client.ID,
 		IssuedAt:  idToken.IssuedAt.Unix(),
 		NotBefore: idToken.IssuedAt.Unix(),
@@ -145,9 +150,9 @@ func (s *Server) introspectAccessToken(ctx context.Context, token string) (*Intr
 }
 
 func (s *Server) handleIntrospect(w http.ResponseWriter, r *http.Request) {
-	var introspect *Introspection
-
 	ctx := r.Context()
+
+	var introspect *Introspection
 	token, tokenType, err := s.getTokenFromRequest(r)
 	if err == nil {
 		switch tokenType {

server/introspectionhandler_test.go

@@ -3,6 +3,7 @@ package server
 import (
 	"context"
 	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -33,12 +34,10 @@ func TestGetTokenFromRequestSuccess(t *testing.T) {
 	var expectedToken = "aaa"
 	var expectedTokenType = "bbb"
 
-	reqBody := strings.NewReader(
+	req := httptest.NewRequest(http.MethodPost, "https://test.tech/introspect", strings.NewReader(
 		fmt.Sprintf(`token=%s&token_type_hint=%s`, expectedToken, expectedTokenType),
-	)
-	req := httptest.NewRequest(http.MethodPost, "https://test.tech/introspect", reqBody)
+	))
 	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	req.Header.Set("Content-Length", fmt.Sprint(reqBody.Len()))
 
 	token, tokenType, err := s.getTokenFromRequest(req)
 	if err != nil {
@@ -81,6 +80,15 @@ func TestGetTokenFromRequestFailure(t *testing.T) {
 		desc: "The POST body can not be empty.",
 		code: http.StatusBadRequest,
 	})
+
+	req := httptest.NewRequest(http.MethodPost, "https://test.tech/introspect", strings.NewReader("token_type_hint=access_token"))
+	req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
+	_, _, err = s.getTokenFromRequest(req)
+	require.ErrorIs(t, err, &introspectionError{
+		typ:  errInvalidRequest,
+		desc: "The POST body doesn't contain 'token' parameter.",
+		code: http.StatusBadRequest,
+	})
 }
 
 func TestIntrospectRefreshToken(t *testing.T) {
@@ -107,30 +115,25 @@ func TestIntrospectRefreshToken(t *testing.T) {
 	require.NoError(t, err)
 	require.EqualValues(t, &Introspection{
 		Active:    true,
-		Scope:     "",
 		ClientID:  "test",
-		Subject:   "1",
+		Subject:   "CgExEgR0ZXN0",
 		Expiry:    t0.Add(s.refreshTokenPolicy.absoluteLifetime).Unix(),
 		IssuedAt:  t0.Unix(),
 		NotBefore: t0.Unix(),
-		Username:  "",
 		Audience: []string{
 			"test",
 		},
-		Issuer:     s.issuerURL.String(),
-		JwtTokenID: "test",
-		TokenType:  "Bearer",
-		TokenUse:   "refresh_token",
+		Issuer:    s.issuerURL.String(),
+		TokenType: "Bearer",
+		TokenUse:  "refresh_token",
 		Extra: IntrospectionExtra{
-			AuthorizingParty: "",
-			Email:            "[email protected]",
-			EmailVerified:    &trueValue,
+			Email:         "[email protected]",
+			EmailVerified: &trueValue,
 			Groups: []string{
 				"a",
 				"b",
 			},
-			Name:              "jane",
-			PreferredUsername: "",
+			Name: "jane",
 		},
 	}, introspection)
 }
@@ -157,38 +160,56 @@ func TestIntrospectAccessToken(t *testing.T) {
 		Email:         "[email protected]",
 		EmailVerified: true,
 		Groups:        []string{"a", "b"},
-	}, []string{"openid", "email", "profile", "groups"}, "foo", "bar", "code", "test")
+	}, []string{"openid", "email", "profile", "groups"}, "foo", "", "", "test")
 	require.NoError(t, err)
 
-	// Nominal test-case
+	// Nominal active test-case
 	introspection, err := s.introspectAccessToken(context.Background(), token)
-	require.NoError(t, err)
+	require.ErrorIs(t, err, nil)
 	require.EqualValues(t, &Introspection{
 		Active:    true,
-		Scope:     "",
 		ClientID:  "test",
 		Subject:   "CgExEgR0ZXN0",
 		Expiry:    expiry.Unix(),
 		IssuedAt:  t0.Unix(),
 		NotBefore: t0.Unix(),
-		Username:  "",
 		Audience: []string{
 			"test",
 		},
-		Issuer:     s.issuerURL.String(),
-		JwtTokenID: "_N4rLtula_QIYB-3If6bXA",
-		TokenType:  "Bearer",
-		TokenUse:   "access_token",
+		Issuer:    s.issuerURL.String(),
+		TokenType: "Bearer",
+		TokenUse:  "access_token",
 		Extra: IntrospectionExtra{
-			AuthorizingParty: "",
-			Email:            "[email protected]",
-			EmailVerified:    &trueValue,
+			Email:         "[email protected]",
+			EmailVerified: &trueValue,
 			Groups: []string{
 				"a",
 				"b",
 			},
-			Name:              "jane",
-			PreferredUsername: "",
+			Name: "jane",
 		},
 	}, introspection)
+
+	// Nominal inactive test-case
+	introspection, err = s.introspectAccessToken(context.Background(), "token")
+	require.ErrorIs(t, err, &introspectionError{
+		typ:  errInactiveToken,
+		code: http.StatusUnauthorized,
+	})
+	require.EqualValues(t, introspection, (*Introspection)(nil))
+}
+
+func TestIntrospectInactiveErr(t *testing.T) {
+	writeRecorder := httptest.NewRecorder()
+	err := introspectInactiveErr(writeRecorder)
+	require.NoError(t, err)
+
+	res := writeRecorder.Result()
+	require.Equal(t, http.StatusUnauthorized, res.StatusCode)
+	require.Equal(t, "application/json", res.Header.Get("Content-Type"))
+
+	data, err := io.ReadAll(res.Body)
+	defer res.Body.Close()
+	require.NoError(t, err)
+	require.Equal(t, `{"active":false}`, string(data))
 }

server/oauth2.go

@@ -344,6 +344,15 @@ func (s *Server) getAudience(clientID string, scopes []string) audience {
 	return aud
 }
 
+func (s *Server) genSubject(userID string, connID string) (string, error) {
+	sub := &internal.IDTokenSubject{
+		UserId: userID,
+		ConnId: connID,
+	}
+
+	return internal.Marshal(sub)
+}
+
 func (s *Server) newIDToken(clientID string, claims storage.Claims, scopes []string, nonce, accessToken, code, connID string) (idToken string, expiry time.Time, err error) {
 	keys, err := s.storage.GetKeys()
 	if err != nil {
@@ -363,12 +372,7 @@ func (s *Server) newIDToken(clientID string, claims storage.Claims, scopes []str
 	issuedAt := s.now()
 	expiry = issuedAt.Add(s.idTokensValidFor)
 
-	sub := &internal.IDTokenSubject{
-		UserId: claims.UserID,
-		ConnId: connID,
-	}
-
-	subjectString, err := internal.Marshal(sub)
+	subjectString, err := s.genSubject(claims.UserID, connID)
 	if err != nil {
 		s.logger.Errorf("failed to marshal offline session ID: %v", err)
 		return "", expiry, fmt.Errorf("failed to marshal offline session ID: %v", err)