mirror of
https://github.com/pocket-id/pocket-id.git
synced 2026-07-15 21:48:13 +03:00
fix: PAR parameters not respected by authorize page
This commit is contained in:
@@ -87,10 +87,13 @@ func RegisterFrontend(router *gin.Engine, oidcService *service.OidcService) erro
|
||||
if isSPARequest(path, distFS) {
|
||||
nonce := middleware.GetCSPNonce(c)
|
||||
|
||||
if isOAuth2AuthorizationPostRequest(c) {
|
||||
// In that case, we need to validate and allow form submissions to the redirect_uri
|
||||
redirectURI := c.Query("redirect_uri")
|
||||
// For an authorization request that responds via response_mode=form_post, the
|
||||
// consent page auto-submits a form to the client's callback, so that callback
|
||||
// must be allowed in the CSP form-action directive
|
||||
isAuthPostRequest, redirectURI := isOAuth2AuthorizationPostRequest(c, oidcService)
|
||||
if isAuthPostRequest {
|
||||
clientID := c.Query("client_id")
|
||||
// In that case, we need to validate and allow form submissions to the redirect_uri
|
||||
validatedRedirectURI, err := oidcService.ResolveAllowedCallbackURL(c.Request.Context(), clientID, redirectURI)
|
||||
if err == nil {
|
||||
c.Header("Content-Security-Policy", middleware.BuildCSP(nonce, validatedRedirectURI))
|
||||
@@ -113,17 +116,21 @@ func RegisterFrontend(router *gin.Engine, oidcService *service.OidcService) erro
|
||||
}
|
||||
|
||||
rateLimitMiddleware := middleware.NewRateLimitMiddleware().Add(rate.Every(300*time.Millisecond), 50)
|
||||
router.NoRoute(rateLimitOnlyForOAuth2AuthorizationPostRequest(rateLimitMiddleware, distFS), handler)
|
||||
router.NoRoute(rateLimitOnlyForOAuth2AuthorizationPostRequest(rateLimitMiddleware, oidcService, distFS), handler)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func rateLimitOnlyForOAuth2AuthorizationPostRequest(rateLimitMiddleware gin.HandlerFunc, distFS fs.FS) gin.HandlerFunc {
|
||||
func rateLimitOnlyForOAuth2AuthorizationPostRequest(rateLimitMiddleware gin.HandlerFunc, oidcService *service.OidcService, distFS fs.FS) gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
path := strings.TrimPrefix(c.Request.URL.Path, "/")
|
||||
if isSPARequest(path, distFS) && isOAuth2AuthorizationPostRequest(c) {
|
||||
rateLimitMiddleware(c)
|
||||
return
|
||||
|
||||
if isSPARequest(path, distFS) {
|
||||
isAuthPostRequest, _ := isOAuth2AuthorizationPostRequest(c, oidcService)
|
||||
if isAuthPostRequest {
|
||||
rateLimitMiddleware(c)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
c.Next()
|
||||
@@ -132,12 +139,29 @@ func rateLimitOnlyForOAuth2AuthorizationPostRequest(rateLimitMiddleware gin.Hand
|
||||
|
||||
// isOAuth2AuthorizationRequest checks if this is an OAuth2 authorization request with response_mode=form_post
|
||||
// In that case, we need to validate and allow form submissions to the redirect_uri
|
||||
func isOAuth2AuthorizationPostRequest(c *gin.Context) bool {
|
||||
func isOAuth2AuthorizationPostRequest(c *gin.Context, oidcService *service.OidcService) (bool, string) {
|
||||
responseMode := c.Query("response_mode")
|
||||
redirectURI := c.Query("redirect_uri")
|
||||
clientID := c.Query("client_id")
|
||||
requestUri := c.Query("request_uri")
|
||||
|
||||
return responseMode == "form_post" && redirectURI != "" && clientID != ""
|
||||
if c.Request.URL.Path != "/authorize" {
|
||||
return false, ""
|
||||
}
|
||||
|
||||
if requestUri != "" && clientID != "" {
|
||||
par, err := oidcService.GetPushedAuthorizationRequest(c.Request.Context(), clientID, requestUri)
|
||||
if err != nil {
|
||||
return false, ""
|
||||
}
|
||||
|
||||
responseMode = par.Parameters.ResponseMode
|
||||
redirectURI = par.Parameters.RedirectURI
|
||||
|
||||
}
|
||||
|
||||
ok := responseMode == "form_post" && redirectURI != "" && clientID != ""
|
||||
return ok, redirectURI
|
||||
}
|
||||
|
||||
func isSPARequest(path string, distFS fs.FS) bool {
|
||||
|
||||
@@ -43,7 +43,7 @@ func TestRateLimitOnlyForOAuth2AuthorizationPostRequest(t *testing.T) {
|
||||
middleware := rateLimitOnlyForOAuth2AuthorizationPostRequest(func(c *gin.Context) {
|
||||
rateLimited = true
|
||||
c.Abort()
|
||||
}, distFS)
|
||||
}, nil, distFS)
|
||||
|
||||
router := gin.New()
|
||||
router.NoRoute(
|
||||
@@ -67,7 +67,7 @@ func TestRateLimitOnlyForOAuth2AuthorizationPostRequest(t *testing.T) {
|
||||
middleware := rateLimitOnlyForOAuth2AuthorizationPostRequest(func(c *gin.Context) {
|
||||
rateLimited = true
|
||||
c.Abort()
|
||||
}, distFS)
|
||||
}, nil, distFS)
|
||||
|
||||
router := gin.New()
|
||||
router.NoRoute(
|
||||
@@ -91,7 +91,7 @@ func TestRateLimitOnlyForOAuth2AuthorizationPostRequest(t *testing.T) {
|
||||
middleware := rateLimitOnlyForOAuth2AuthorizationPostRequest(func(c *gin.Context) {
|
||||
rateLimited = true
|
||||
c.Abort()
|
||||
}, distFS)
|
||||
}, nil, distFS)
|
||||
|
||||
router := gin.New()
|
||||
router.NoRoute(
|
||||
@@ -108,4 +108,30 @@ func TestRateLimitOnlyForOAuth2AuthorizationPostRequest(t *testing.T) {
|
||||
assert.False(t, rateLimited)
|
||||
assert.True(t, nextCalled)
|
||||
})
|
||||
|
||||
t.Run("does not rate limit non-authorize spa path with form_post params", func(t *testing.T) {
|
||||
rateLimited := false
|
||||
nextCalled := false
|
||||
// oidcService is nil: a non-/authorize path is rejected by the path guard
|
||||
// before the request_uri branch that would dereference it.
|
||||
middleware := rateLimitOnlyForOAuth2AuthorizationPostRequest(func(c *gin.Context) {
|
||||
rateLimited = true
|
||||
c.Abort()
|
||||
}, nil, distFS)
|
||||
|
||||
router := gin.New()
|
||||
router.NoRoute(
|
||||
middleware,
|
||||
func(c *gin.Context) {
|
||||
nextCalled = true
|
||||
},
|
||||
)
|
||||
|
||||
recorder := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/settings?response_mode=form_post&client_id=test&redirect_uri=https://example.com/callback", nil)
|
||||
router.ServeHTTP(recorder, req)
|
||||
|
||||
assert.False(t, rateLimited)
|
||||
assert.True(t, nextCalled)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -33,6 +33,7 @@ func NewOidcController(group *gin.RouterGroup, authMiddleware *middleware.AuthMi
|
||||
|
||||
group.POST("/oidc/authorize", authMiddleware.WithAdminNotRequired().Add(), oc.authorizeHandler)
|
||||
group.POST("/oidc/authorization-required", authMiddleware.WithAdminNotRequired().Add(), oc.authorizationConfirmationRequiredHandler)
|
||||
group.GET("/oidc/par-request-info", authMiddleware.WithAdminNotRequired().Add(), oc.parRequestInfoHandler)
|
||||
|
||||
group.POST("/oidc/token", oc.createTokensHandler)
|
||||
group.POST("/oidc/par", oc.pushedAuthorizationRequestHandler)
|
||||
@@ -164,6 +165,43 @@ func (oc *OidcController) authorizationConfirmationRequiredHandler(c *gin.Contex
|
||||
c.JSON(http.StatusOK, gin.H{"authorizationRequired": authorizationRequired, "scope": scope})
|
||||
}
|
||||
|
||||
// parRequestInfoHandler godoc
|
||||
// @Summary Resolve stored authorization request parameters
|
||||
// @Description Resolve the parameters of a stored request_uri request so the consent page can render the requested scope and build the final redirect. Does not consume the request.
|
||||
// @Tags OIDC
|
||||
// @Produce json
|
||||
// @Param client_id query string true "Client ID"
|
||||
// @Param request_uri query string true "Request URI returned from the PAR endpoint"
|
||||
// @Success 200 {object} dto.OidcAuthorizeRequestInfoDto "Resolved authorization request parameters"
|
||||
// @Router /api/oidc/par-request-info [get]
|
||||
func (oc *OidcController) parRequestInfoHandler(c *gin.Context) {
|
||||
clientID := c.Query("client_id")
|
||||
requestURI := c.Query("request_uri")
|
||||
if clientID == "" {
|
||||
_ = c.Error(&common.ValidationError{Message: "client_id is required"})
|
||||
return
|
||||
}
|
||||
if requestURI == "" {
|
||||
_ = c.Error(&common.ValidationError{Message: "request_uri is required"})
|
||||
return
|
||||
}
|
||||
|
||||
info, err := oc.oidcService.GetPushedAuthorizationRequest(c.Request.Context(), clientID, requestURI)
|
||||
if err != nil {
|
||||
_ = c.Error(err)
|
||||
return
|
||||
}
|
||||
|
||||
var infoDto dto.OidcAuthorizeRequestInfoDto
|
||||
err = dto.MapStruct(info.Parameters, &infoDto)
|
||||
if err != nil {
|
||||
_ = c.Error(err)
|
||||
return
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, infoDto)
|
||||
}
|
||||
|
||||
// createTokensHandler godoc
|
||||
// @Summary Create OIDC tokens
|
||||
// @Description Exchange authorization code or refresh token for access tokens
|
||||
|
||||
@@ -91,6 +91,7 @@ type OidcPARRequestDto struct {
|
||||
CodeChallenge string `form:"code_challenge"`
|
||||
CodeChallengeMethod string `form:"code_challenge_method"`
|
||||
Prompt string `form:"prompt"`
|
||||
ResponseMode string `form:"response_mode" binding:"omitempty,response_mode"`
|
||||
}
|
||||
|
||||
type OidcPARResponseDto struct {
|
||||
@@ -110,6 +111,15 @@ type AuthorizationRequiredDto struct {
|
||||
RequestURI string `json:"requestURI"`
|
||||
}
|
||||
|
||||
type OidcAuthorizeRequestInfoDto struct {
|
||||
Scope string `json:"scope"`
|
||||
RedirectURI string `json:"redirectURI"`
|
||||
State string `json:"state,omitempty"`
|
||||
Nonce string `json:"nonce,omitempty"`
|
||||
ResponseMode string `json:"responseMode,omitempty"`
|
||||
Prompt string `json:"prompt,omitempty"`
|
||||
}
|
||||
|
||||
type OidcCreateTokensDto struct {
|
||||
GrantType string `form:"grant_type" binding:"required"`
|
||||
Code string `form:"code"`
|
||||
|
||||
@@ -162,15 +162,28 @@ type OidcDeviceCode struct {
|
||||
type OidcPushedAuthorizationRequest struct {
|
||||
Base
|
||||
|
||||
RequestURI string
|
||||
ClientID string
|
||||
Scope string
|
||||
RedirectURI string
|
||||
State string
|
||||
Nonce string
|
||||
CodeChallenge *string
|
||||
CodeChallengeMethod *string
|
||||
ResponseType string
|
||||
Prompt string
|
||||
ExpiresAt datatype.DateTime
|
||||
RequestURI string
|
||||
ClientID string
|
||||
Parameters OidcAuthorizationRequestParameters
|
||||
ExpiresAt datatype.DateTime
|
||||
}
|
||||
|
||||
type OidcAuthorizationRequestParameters struct { //nolint:recvcheck
|
||||
Scope string `json:"scope,omitempty"`
|
||||
RedirectURI string `json:"redirect_uri,omitempty"`
|
||||
State string `json:"state,omitempty"`
|
||||
Nonce string `json:"nonce,omitempty"`
|
||||
CodeChallenge string `json:"code_challenge,omitempty"`
|
||||
CodeChallengeMethod string `json:"code_challenge_method,omitempty"`
|
||||
ResponseType string `json:"response_type,omitempty"`
|
||||
Prompt string `json:"prompt,omitempty"`
|
||||
ResponseMode string `json:"response_mode,omitempty"`
|
||||
}
|
||||
|
||||
func (p *OidcAuthorizationRequestParameters) Scan(value any) error {
|
||||
return utils.UnmarshalJSONFromDatabase(p, value)
|
||||
}
|
||||
|
||||
func (p OidcAuthorizationRequestParameters) Value() (driver.Value, error) {
|
||||
return json.Marshal(p)
|
||||
}
|
||||
|
||||
@@ -259,25 +259,18 @@ func (s *OidcService) Authorize(ctx context.Context, input dto.AuthorizeOidcClie
|
||||
// applyPushedAuthorizationRequest consumes the stored PAR for the given request_uri
|
||||
// and overwrites the corresponding fields on input.
|
||||
func (s *OidcService) applyPushedAuthorizationRequest(ctx context.Context, tx *gorm.DB, input *dto.AuthorizeOidcClientRequestDto) error {
|
||||
par, err := s.getAndConsumePushedAuthorizationRequest(ctx, tx, input.ClientID, input.RequestURI)
|
||||
parMeta, err := s.getAndConsumePushedAuthorizationRequest(ctx, tx, input.ClientID, input.RequestURI)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
par := parMeta.Parameters
|
||||
|
||||
input.Scope = par.Scope
|
||||
input.CallbackURL = par.RedirectURI
|
||||
input.Nonce = par.Nonce
|
||||
input.Prompt = par.Prompt
|
||||
|
||||
input.CodeChallenge = ""
|
||||
if par.CodeChallenge != nil {
|
||||
input.CodeChallenge = *par.CodeChallenge
|
||||
}
|
||||
|
||||
input.CodeChallengeMethod = ""
|
||||
if par.CodeChallengeMethod != nil {
|
||||
input.CodeChallengeMethod = *par.CodeChallengeMethod
|
||||
}
|
||||
input.CodeChallenge = par.CodeChallenge
|
||||
input.CodeChallengeMethod = par.CodeChallengeMethod
|
||||
|
||||
return nil
|
||||
}
|
||||
@@ -288,15 +281,13 @@ func (s *OidcService) HasAuthorizedClient(ctx context.Context, clientID, userID,
|
||||
}
|
||||
|
||||
// AuthorizationRequired reports whether the user must confirm authorization for the client.
|
||||
// In the PAR flow (requestURI set), the scope is resolved from the stored request without
|
||||
// consuming it, so it is also returned so the consent screen can render the requested scopes.
|
||||
func (s *OidcService) AuthorizationRequired(ctx context.Context, clientID, userID, scope, requestURI string) (required bool, resolvedScope string, err error) {
|
||||
if requestURI != "" {
|
||||
par, err := s.getPushedAuthorizationRequest(ctx, s.db, clientID, requestURI)
|
||||
par, err := s.getPushedAuthorizationRequestInternal(ctx, s.db, clientID, requestURI)
|
||||
if err != nil {
|
||||
return false, "", err
|
||||
}
|
||||
scope = par.Scope
|
||||
scope = par.Parameters.Scope
|
||||
}
|
||||
|
||||
hasAuthorized, err := s.hasAuthorizedClientInternal(ctx, clientID, userID, scope, s.db)
|
||||
@@ -1410,27 +1401,21 @@ func (s *OidcService) CreatePushedAuthorizationRequest(ctx context.Context, cred
|
||||
}
|
||||
requestURI = parRequestURIPrefix + randomSuffix
|
||||
|
||||
var codeChallenge *string
|
||||
var codeChallengeMethod *string
|
||||
if input.CodeChallenge != "" {
|
||||
codeChallenge = &input.CodeChallenge
|
||||
}
|
||||
if input.CodeChallengeMethod != "" {
|
||||
codeChallengeMethod = &input.CodeChallengeMethod
|
||||
}
|
||||
|
||||
par := model.OidcPushedAuthorizationRequest{
|
||||
RequestURI: requestURI,
|
||||
ClientID: client.ID,
|
||||
Scope: input.Scope,
|
||||
RedirectURI: input.RedirectURI,
|
||||
State: input.State,
|
||||
Nonce: input.Nonce,
|
||||
CodeChallenge: codeChallenge,
|
||||
CodeChallengeMethod: codeChallengeMethod,
|
||||
ResponseType: input.ResponseType,
|
||||
Prompt: input.Prompt,
|
||||
ExpiresAt: datatype.DateTime(time.Now().Add(PARDuration)),
|
||||
RequestURI: requestURI,
|
||||
ClientID: client.ID,
|
||||
ExpiresAt: datatype.DateTime(time.Now().Add(PARDuration)),
|
||||
Parameters: model.OidcAuthorizationRequestParameters{
|
||||
Scope: input.Scope,
|
||||
RedirectURI: input.RedirectURI,
|
||||
State: input.State,
|
||||
Nonce: input.Nonce,
|
||||
CodeChallenge: input.CodeChallenge,
|
||||
CodeChallengeMethod: input.CodeChallengeMethod,
|
||||
ResponseType: input.ResponseType,
|
||||
Prompt: input.Prompt,
|
||||
ResponseMode: input.ResponseMode,
|
||||
},
|
||||
}
|
||||
|
||||
if err = s.db.WithContext(ctx).Create(&par).Error; err != nil {
|
||||
@@ -1440,8 +1425,13 @@ func (s *OidcService) CreatePushedAuthorizationRequest(ctx context.Context, cred
|
||||
return requestURI, int(PARDuration.Seconds()), nil
|
||||
}
|
||||
|
||||
// getPushedAuthorizationRequest retrieves a PAR record without consuming it.
|
||||
func (s *OidcService) getPushedAuthorizationRequest(ctx context.Context, tx *gorm.DB, clientID, requestURI string) (model.OidcPushedAuthorizationRequest, error) {
|
||||
// GetPushedAuthorizationRequest retrieves a PAR record without consuming it.
|
||||
func (s *OidcService) GetPushedAuthorizationRequest(ctx context.Context, clientID, requestURI string) (model.OidcPushedAuthorizationRequest, error) {
|
||||
return s.getPushedAuthorizationRequestInternal(ctx, s.db, clientID, requestURI)
|
||||
}
|
||||
|
||||
// getPushedAuthorizationRequestInternal retrieves a PAR record without consuming it.
|
||||
func (s *OidcService) getPushedAuthorizationRequestInternal(ctx context.Context, tx *gorm.DB, clientID, requestURI string) (model.OidcPushedAuthorizationRequest, error) {
|
||||
var par model.OidcPushedAuthorizationRequest
|
||||
err := tx.
|
||||
WithContext(ctx).
|
||||
|
||||
@@ -3,14 +3,7 @@ CREATE TABLE oidc_pushed_authorization_requests (
|
||||
created_at TIMESTAMPTZ NOT NULL,
|
||||
request_uri TEXT NOT NULL UNIQUE,
|
||||
client_id TEXT NOT NULL REFERENCES oidc_clients(id) ON DELETE CASCADE,
|
||||
scope TEXT NOT NULL DEFAULT '',
|
||||
redirect_uri TEXT NOT NULL DEFAULT '',
|
||||
state TEXT NOT NULL DEFAULT '',
|
||||
nonce TEXT NOT NULL DEFAULT '',
|
||||
code_challenge TEXT,
|
||||
code_challenge_method TEXT,
|
||||
response_type TEXT NOT NULL DEFAULT 'code',
|
||||
prompt TEXT NOT NULL DEFAULT '',
|
||||
parameters JSONB NOT NULL DEFAULT '{}',
|
||||
expires_at TIMESTAMPTZ NOT NULL
|
||||
);
|
||||
|
||||
|
||||
@@ -3,14 +3,7 @@ CREATE TABLE oidc_pushed_authorization_requests (
|
||||
created_at INTEGER NOT NULL,
|
||||
request_uri TEXT NOT NULL UNIQUE,
|
||||
client_id TEXT NOT NULL REFERENCES oidc_clients(id) ON DELETE CASCADE,
|
||||
scope TEXT NOT NULL DEFAULT '',
|
||||
redirect_uri TEXT NOT NULL DEFAULT '',
|
||||
state TEXT NOT NULL DEFAULT '',
|
||||
nonce TEXT NOT NULL DEFAULT '',
|
||||
code_challenge TEXT,
|
||||
code_challenge_method TEXT,
|
||||
response_type TEXT NOT NULL DEFAULT 'code',
|
||||
prompt TEXT NOT NULL DEFAULT '',
|
||||
parameters TEXT NOT NULL DEFAULT '{}',
|
||||
expires_at INTEGER NOT NULL
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user