fix token validation and error messages

pkg/op/authrequest.go

@@ -2,7 +2,6 @@ package op
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -95,19 +94,19 @@ func ValidateAuthRequest(ctx context.Context, authReq *oidc.AuthRequest, storage
 
 func ValidateAuthReqScopes(scopes []string) error {
 	if len(scopes) == 0 {
-		return ErrInvalidRequest("Unforuntately, the scope of your request is missing. Please ensure your scope value is not 0, and try again. If you have any questions, you may contact the administrator of the application.")
+		return ErrInvalidRequest("Unfortunately, the scope parameter of your request is missing. Please ensure your scope value is not empty, and try again. If you have any questions, you may contact the administrator of the application.")
 	}
 	if !utils.Contains(scopes, oidc.ScopeOpenID) {
-		return ErrInvalidRequest("Unfortunately, the scope openid of your request is missing. Please ensure your scope openid is complete and accurate, and try again. If you have any questions, you may contact the administrator of the application.")
+		return ErrInvalidRequest("Unfortunately, the scope `openid` is missing. Please ensure your scope configuration is correct (containing the `openid` value), and try again. If you have any questions, you may contact the administrator of the application.")
 	}
 	return nil
 }
 
-func ValidateAuthReqRedirectURI(ctx context.Context, uri, client_id string, responseType oidc.ResponseType, storage OPStorage) error {
+func ValidateAuthReqRedirectURI(ctx context.Context, uri, clientID string, responseType oidc.ResponseType, storage OPStorage) error {
 	if uri == "" {
 		return ErrInvalidRequestRedirectURI("Unfortunately, the client's redirect_uri is missing. Please ensure your redirect_uri is included in the request, and try again. If you have any questions, you may contact the administrator of the application.")
 	}
-	client, err := storage.GetClientByClientID(ctx, client_id)
+	client, err := storage.GetClientByClientID(ctx, clientID)
 	if err != nil {
 		return ErrServerError(err.Error())
 	}
@@ -143,9 +142,9 @@ func ValidateAuthReqResponseType(responseType oidc.ResponseType) error {
 		oidc.ResponseTypeIDTokenOnly:
 		return nil
 	case "":
-		return ErrInvalidRequest("Unfortunately, a response type is missing in your request. Please ensure the response type is complete and accurate, and try again. If you have any questions, you may contact the administrator of the application.")
+		return ErrInvalidRequest("Unfortunately, the response type is missing in your request. Please ensure the response type is complete and accurate, and try again. If you have any questions, you may contact the administrator of the application.")
 	default:
-		return ErrInvalidRequest("response_type invalid")
+		return ErrInvalidRequest("Unfortunately, the response type provided in your request is invalid. Please ensure the response type is valid, and try again. If you have any questions, you may contact the administrator of the application.")
 	}
 }
 
@@ -153,7 +152,7 @@ func ValidateAuthReqIDTokenHint(ctx context.Context, idTokenHint string, verifie
 	if idTokenHint == "" {
 		return "", nil
 	}
-	claims, err := verifier.Verify(ctx, "", idTokenHint)
+	claims, err := verifier.VerifyIdToken(ctx, idTokenHint)
 	if err != nil {
 		return "", ErrInvalidRequest("Unfortunately, the id_token_hint is invalid. Please ensure the id_token_hint is complete and accurate, and try again. If you have any questions, you may contact the administrator of the application.")
 	}
@@ -175,7 +174,7 @@ func AuthorizeCallback(w http.ResponseWriter, r *http.Request, authorizer Author
 		return
 	}
 	if !authReq.Done() {
-		AuthRequestError(w, r, authReq, errors.New("user not logged in"), authorizer.Encoder())
+		AuthRequestError(w, r, authReq, ErrInteractionRequired("Unfortunately, the user may is not logged in and/or additional interaction is required."), authorizer.Encoder())
 		return
 	}
 	AuthResponse(authReq, authorizer, w, r)

pkg/op/config_test.go

@@ -54,9 +54,9 @@ func TestValidateIssuer(t *testing.T) {
 			false,
 		},
 		{
-			"localhost with http ok",
+			"localhost with http fails",
 			args{"http://localhost:9999"},
-			false,
+			true,
 		},
 	}
 	for _, tt := range tests {
@@ -78,7 +78,7 @@ func TestValidateIssuerDevLocalAllowed(t *testing.T) {
 		wantErr bool
 	}{
 		{
-			"localhost with http ok",
+			"localhost with http with dev ok",
 			args{"http://localhost:9999"},
 			false,
 		},

pkg/op/error.go

@@ -10,6 +10,8 @@ import (
 
 const (
 	InvalidRequest      errorType = "invalid_request"
+	InvalidRequestUri   errorType = "invalid_request_uri"
+	InteractionRequired errorType = "interaction_required"
 	ServerError         errorType = "server_error"
 )
 
@@ -22,11 +24,17 @@ var (
 	}
 	ErrInvalidRequestRedirectURI = func(description string) *OAuthError {
 		return &OAuthError{
-			ErrorType:        InvalidRequest,
+			ErrorType:        InvalidRequestUri,
 			Description:      description,
 			redirectDisabled: true,
 		}
 	}
+	ErrInteractionRequired = func(description string) *OAuthError {
+		return &OAuthError{
+			ErrorType:   InteractionRequired,
+			Description: description,
+		}
+	}
 	ErrServerError = func(description string) *OAuthError {
 		return &OAuthError{
 			ErrorType:   ServerError,
@@ -54,7 +62,7 @@ func AuthRequestError(w http.ResponseWriter, r *http.Request, authReq ErrAuthReq
 		e.ErrorType = ServerError
 		e.Description = err.Error()
 	}
-	e.state = authReq.GetState()
+	e.State = authReq.GetState()
 	if authReq.GetRedirectURI() == "" || e.redirectDisabled {
 		http.Error(w, e.Description, http.StatusBadRequest)
 		return
@@ -87,9 +95,9 @@ func RequestError(w http.ResponseWriter, r *http.Request, err error) {
 
 type OAuthError struct {
 	ErrorType        errorType `json:"error" schema:"error"`
-	Description      string    `json:"error_description" schema:"error_description"`
+	Description      string    `json:"error_description,omitempty" schema:"error_description,omitempty"`
-	state            string    `json:"state" schema:"state"`
+	State            string    `json:"state,omitempty" schema:"state,omitempty"`
-	redirectDisabled bool
+	redirectDisabled bool      `json:"-" schema:"-"`
 }
 
 func (e *OAuthError) Error() string {

pkg/op/session.go

@@ -57,7 +57,7 @@ func ValidateEndSessionRequest(ctx context.Context, req *oidc.EndSessionRequest,
 	if req.IdTokenHint == "" {
 		return session, nil
 	}
-	claims, err := ender.IDTokenVerifier().Verify(ctx, "", req.IdTokenHint)
+	claims, err := ender.IDTokenVerifier().VerifyIdToken(ctx, req.IdTokenHint)
 	if err != nil {
 		return nil, ErrInvalidRequest("id_token_hint invalid")
 	}

pkg/rp/default_verifier.go

@@ -148,7 +148,7 @@ func DefaultACRVerifier(possibleValues []string) ACRVerifier {
 //and https://openid.net/specs/openid-connect-core-1_0.html#CodeFlowTokenValidation
 func (v *DefaultVerifier) Verify(ctx context.Context, accessToken, idTokenString string) (*oidc.IDTokenClaims, error) {
 	v.config.now = time.Now().UTC()
-	idToken, err := v.VerifyIDToken(ctx, idTokenString)
+	idToken, err := v.VerifyIdToken(ctx, idTokenString)
 	if err != nil {
 		return nil, err
 	}
@@ -158,15 +158,9 @@ func (v *DefaultVerifier) Verify(ctx context.Context, accessToken, idTokenString
 	return idToken, nil
 }
 
-func (v *DefaultVerifier) now() time.Time {
+//Verify implements the `VerifyIdToken` method of the `Verifier` interface
-	if v.config.now.IsZero() {
+//according to https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
-		v.config.now = time.Now().UTC().Round(time.Second)
+func (v *DefaultVerifier) VerifyIdToken(ctx context.Context, idTokenString string) (*oidc.IDTokenClaims, error) {
-	}
-	return v.config.now
-}
-
-//VerifyIDToken: https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
-func (v *DefaultVerifier) VerifyIDToken(ctx context.Context, idTokenString string) (*oidc.IDTokenClaims, error) {
 	//1. if encrypted --> decrypt
 	decrypted, err := v.decryptToken(idTokenString)
 	if err != nil {
@@ -227,6 +221,13 @@ func (v *DefaultVerifier) VerifyIDToken(ctx context.Context, idTokenString strin
 	return claims, nil
 }
 
+func (v *DefaultVerifier) now() time.Time {
+	if v.config.now.IsZero() {
+		v.config.now = time.Now().UTC().Round(time.Second)
+	}
+	return v.config.now
+}
+
 func (v *DefaultVerifier) parseToken(tokenString string) (*oidc.IDTokenClaims, []byte, error) {
 	parts := strings.Split(tokenString, ".")
 	if len(parts) != 3 {
@@ -372,7 +373,7 @@ func (v *DefaultVerifier) decryptToken(tokenString string) (string, error) {
 }
 
 func (v *DefaultVerifier) verifyAccessToken(accessToken, atHash string, sigAlgorithm jose.SignatureAlgorithm) error {
-	if accessToken == "" {
+	if atHash == "" {
 		return nil
 	}
 

pkg/rp/mock/verifier.mock.go

@@ -48,3 +48,18 @@ func (mr *MockVerifierMockRecorder) Verify(arg0, arg1, arg2 interface{}) *gomock
 	mr.mock.ctrl.T.Helper()
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Verify", reflect.TypeOf((*MockVerifier)(nil).Verify), arg0, arg1, arg2)
 }
+
+// VerifyIdToken mocks base method
+func (m *MockVerifier) VerifyIdToken(arg0 context.Context, arg1 string) (*oidc.IDTokenClaims, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "VerifyIdToken", arg0, arg1)
+	ret0, _ := ret[0].(*oidc.IDTokenClaims)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// VerifyIdToken indicates an expected call of VerifyIdToken
+func (mr *MockVerifierMockRecorder) VerifyIdToken(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VerifyIdToken", reflect.TypeOf((*MockVerifier)(nil).VerifyIdToken), arg0, arg1)
+}

pkg/rp/verifier.go

@@ -12,4 +12,7 @@ type Verifier interface {
 
 	//Verify checks the access_token and id_token and returns the `id token claims`
 	Verify(ctx context.Context, accessToken, idTokenString string) (*oidc.IDTokenClaims, error)
+
+	//VerifyIdToken checks the id_token only and returns its `id token claims`
+	VerifyIdToken(ctx context.Context, idTokenString string) (*oidc.IDTokenClaims, error)
 }