mirror of
https://github.com/pocket-id/pocket-id.git
synced 2026-10-18 23:59:04 +02:00
feat: add ability to deselect scopes on authorization
This commit is contained in:
@@ -128,7 +128,7 @@ func (h *authorizationHandler) completeInteraction(c *gin.Context) {
|
||||
}
|
||||
|
||||
reauthenticationToken, _ := c.Cookie(cookie.ReauthenticationTokenCookieName)
|
||||
response, err := h.authorizationService.completeInteractionStep(c.Request.Context(), interactionID, principal.UserID, request.Step, reauthenticationToken, principal.AuthenticationTime, requestMetaFromGin(c))
|
||||
response, err := h.authorizationService.completeInteractionStep(c.Request.Context(), interactionID, principal.UserID, request.Step, request.Scopes, reauthenticationToken, principal.AuthenticationTime, requestMetaFromGin(c))
|
||||
if err != nil {
|
||||
_ = c.Error(err)
|
||||
return
|
||||
|
||||
@@ -581,7 +581,7 @@ func (s *authorizationService) resolveScopeInfoForRequest(ctx context.Context, r
|
||||
return infos, nil
|
||||
}
|
||||
|
||||
func (s *authorizationService) completeInteractionStep(ctx context.Context, interactionSessionID, userID string, step interactionStep, reauthenticationToken string, authenticationTime time.Time, meta requestMeta) (completeInteractionResponse, error) {
|
||||
func (s *authorizationService) completeInteractionStep(ctx context.Context, interactionSessionID, userID string, step interactionStep, approvedScopes []string, reauthenticationToken string, authenticationTime time.Time, meta requestMeta) (completeInteractionResponse, error) {
|
||||
var interactionSession InteractionSession
|
||||
var response completeInteractionResponse
|
||||
err := withTx(ctx, s.db, func(ctx context.Context) error {
|
||||
@@ -614,7 +614,7 @@ func (s *authorizationService) completeInteractionStep(ctx context.Context, inte
|
||||
return apperror.ValidationMessage("expected interaction step " + string(requiredSteps[0]) + " but got " + string(step))
|
||||
}
|
||||
|
||||
if err := s.applyInteractionStep(ctx, &interactionSession, userID, step, reauthenticationToken, authenticationTime, meta); err != nil {
|
||||
if err := s.applyInteractionStep(ctx, &interactionSession, userID, step, approvedScopes, reauthenticationToken, authenticationTime, meta); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -639,7 +639,7 @@ func (s *authorizationService) completeInteractionStep(ctx context.Context, inte
|
||||
return completeInteractionResponse{Interaction: &interaction}, nil
|
||||
}
|
||||
|
||||
func (s *authorizationService) applyInteractionStep(ctx context.Context, interactionSession *InteractionSession, userID string, step interactionStep, reauthenticationToken string, authenticationTime time.Time, meta requestMeta) error {
|
||||
func (s *authorizationService) applyInteractionStep(ctx context.Context, interactionSession *InteractionSession, userID string, step interactionStep, approvedScopes []string, reauthenticationToken string, authenticationTime time.Time, meta requestMeta) error {
|
||||
switch step {
|
||||
case interactionStepAuthenticate:
|
||||
if err := bindInteractionSessionUser(interactionSession, userID); err != nil {
|
||||
@@ -656,7 +656,7 @@ func (s *authorizationService) applyInteractionStep(ctx context.Context, interac
|
||||
case interactionStepReauthenticate:
|
||||
return s.completeReauthenticationStep(ctx, interactionSession, userID, reauthenticationToken)
|
||||
case interactionStepConsent:
|
||||
return s.completeConsentStep(ctx, interactionSession, userID, meta)
|
||||
return s.completeConsentStep(ctx, interactionSession, userID, approvedScopes, meta)
|
||||
default:
|
||||
return apperror.ValidationMessage("unknown interaction step " + string(step))
|
||||
}
|
||||
@@ -679,10 +679,17 @@ func (s *authorizationService) completeReauthenticationStep(ctx context.Context,
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *authorizationService) completeConsentStep(ctx context.Context, interactionSession *InteractionSession, userID string, meta requestMeta) error {
|
||||
func (s *authorizationService) completeConsentStep(ctx context.Context, interactionSession *InteractionSession, userID string, approvedScopes []string, meta requestMeta) error {
|
||||
if err := bindInteractionSessionUser(interactionSession, userID); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Narrow the request to the scopes the user approved before recording consent, so deselected scopes are neither consented to nor granted
|
||||
if err := narrowInteractionScopes(interactionSession, approvedScopes); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Record consent for the approved scopes only
|
||||
resource := interactionSession.Parameters["resource"]
|
||||
_, _, consentKeys, err := s.resolveGrant(ctx, interactionSession.ClientID, resource, interactionSession.Scopes)
|
||||
if err != nil {
|
||||
@@ -699,6 +706,56 @@ func (s *authorizationService) completeConsentStep(ctx context.Context, interact
|
||||
return nil
|
||||
}
|
||||
|
||||
// requiredConsentScopes are the scopes a user cannot deselect on the consent screen because the client cannot sign the user in without them
|
||||
var requiredConsentScopes = []string{"openid"}
|
||||
|
||||
// narrowApprovedScopes restricts the requested scopes to the ones the user approved on a consent screen
|
||||
// The browser and device consent flows share it so both apply the same rules to a selection
|
||||
// A nil approval keeps every requested scope, so callers that send no selection keep the all-or-nothing behavior
|
||||
func narrowApprovedScopes(requestedScopes, approvedScopes []string) ([]string, error) {
|
||||
if approvedScopes == nil {
|
||||
return requestedScopes, nil
|
||||
}
|
||||
|
||||
// The approval may only remove scopes, never add ones the client did not request
|
||||
for _, scope := range approvedScopes {
|
||||
if !slices.Contains(requestedScopes, scope) {
|
||||
return nil, apperror.ValidationMessage("scope " + scope + " was not requested")
|
||||
}
|
||||
}
|
||||
|
||||
// Keep the requested order so the narrowed request only differs from the original by the removed scopes
|
||||
narrowed := make([]string, 0, len(requestedScopes))
|
||||
for _, scope := range requestedScopes {
|
||||
if slices.Contains(approvedScopes, scope) || slices.Contains(requiredConsentScopes, scope) {
|
||||
narrowed = append(narrowed, scope)
|
||||
}
|
||||
}
|
||||
|
||||
return narrowed, nil
|
||||
}
|
||||
|
||||
// narrowInteractionScopes restricts the interaction session to the scopes the user approved on the consent screen
|
||||
// The stored scope parameter is rewritten too because the resumed authorize request is rebuilt from it, which keeps deselected scopes out of the granted scopes and the issued tokens
|
||||
func narrowInteractionScopes(interactionSession *InteractionSession, approvedScopes []string) error {
|
||||
if approvedScopes == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
narrowed, err := narrowApprovedScopes(interactionSession.Scopes, approvedScopes)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
interactionSession.Scopes = narrowed
|
||||
|
||||
if interactionSession.Parameters == nil {
|
||||
interactionSession.Parameters = InteractionSessionParameters{}
|
||||
}
|
||||
interactionSession.Parameters["scope"] = strings.Join(narrowed, " ")
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *authorizationService) populatePostAuthenticationRequirements(ctx context.Context, userID string, interactionSession *InteractionSession, authenticationTime time.Time) error {
|
||||
if userID == "" {
|
||||
return errors.New("user is required to complete authentication")
|
||||
|
||||
@@ -165,7 +165,7 @@ func TestAuthorizationServiceConsentStepLogsNewClientAuthorization(t *testing.T)
|
||||
Parameters: map[string]string{},
|
||||
}).Error)
|
||||
|
||||
response, err := service.completeInteractionStep(t.Context(), interactionID, userID, interactionStepConsent, "", time.Now().UTC(), requestMeta{})
|
||||
response, err := service.completeInteractionStep(t.Context(), interactionID, userID, interactionStepConsent, nil, "", time.Now().UTC(), requestMeta{})
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, response.RedirectURL)
|
||||
|
||||
@@ -173,6 +173,147 @@ func TestAuthorizationServiceConsentStepLogsNewClientAuthorization(t *testing.T)
|
||||
require.Equal(t, auditlogs.Data{"clientName": "Test Client"}, auditLogger.data[0])
|
||||
}
|
||||
|
||||
// TestAuthorizationServiceConsentStepGrantsOnlyApprovedScopes guards that scopes the user deselects on the consent screen are neither consented to nor granted when the request resumes
|
||||
func TestAuthorizationServiceConsentStepGrantsOnlyApprovedScopes(t *testing.T) {
|
||||
db := testutils.NewDatabaseForTest(t)
|
||||
|
||||
const (
|
||||
userID = "test-user"
|
||||
clientID = "test-client"
|
||||
interactionID = "test-interaction"
|
||||
api = "https://api.example.com"
|
||||
)
|
||||
|
||||
service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil, appconfig.StaticResolver{}), nil, nil, userAccess(map[string][]string{
|
||||
api: {"users.read", "users.write"},
|
||||
}))
|
||||
|
||||
require.NoError(t, db.Create(&model.User{Base: model.Base{ID: userID}}).Error)
|
||||
require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: clientID}, Name: "Test Client"}).Error)
|
||||
require.NoError(t, db.Create(&InteractionSession{
|
||||
Base: model.Base{ID: interactionID},
|
||||
Scopes: datatype.StringList{"openid", "email", "users.read", "users.write"},
|
||||
ClientID: clientID,
|
||||
ConsentRequired: true,
|
||||
RequestedAt: datatype.DateTime(time.Now().UTC()),
|
||||
Parameters: map[string]string{"scope": "openid email users.read users.write", "resource": api},
|
||||
}).Error)
|
||||
|
||||
// The user deselects users.write and does not send openid, which must still be kept
|
||||
response, err := service.completeInteractionStep(t.Context(), interactionID, userID, interactionStepConsent, []string{"email", "users.read"}, "", time.Now().UTC(), requestMeta{})
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, response.RedirectURL)
|
||||
|
||||
// The interaction session and the parameters the resumed request is rebuilt from only carry the approved scopes
|
||||
var interactionSession InteractionSession
|
||||
require.NoError(t, db.First(&interactionSession, "id = ?", interactionID).Error)
|
||||
require.Equal(t, datatype.StringList{"openid", "email", "users.read"}, interactionSession.Scopes)
|
||||
require.Equal(t, "openid email users.read", interactionSession.Parameters["scope"])
|
||||
|
||||
// Consent is only recorded for the approved scopes
|
||||
var authorizedClient model.UserAuthorizedOidcClient
|
||||
require.NoError(t, db.First(&authorizedClient, "user_id = ? AND client_id = ?", userID, clientID).Error)
|
||||
require.Contains(t, authorizedClient.Scope, consentScopeKey(api, "users.read"))
|
||||
require.NotContains(t, authorizedClient.Scope, consentScopeKey(api, "users.write"))
|
||||
|
||||
authorize := func(scopes ...string) (fosite.AuthorizeRequester, authorizationResult, error) {
|
||||
requester := newTestAuthorizeRequesterWithForm("resumed-request", clientID, url.Values{"resource": {api}})
|
||||
requester.(*fosite.AuthorizeRequest).RequestedScope = fosite.Arguments(scopes)
|
||||
authorization, err := service.authorize(t.Context(), authorizeInput{
|
||||
userID: userID,
|
||||
authenticationTime: time.Now().UTC(),
|
||||
requester: requester,
|
||||
interactionID: interactionID,
|
||||
})
|
||||
return requester, authorization, err
|
||||
}
|
||||
|
||||
// Replaying the original scopes must not grant the deselected one
|
||||
_, _, err = authorize("openid", "email", "users.read", "users.write")
|
||||
require.ErrorIs(t, err, fosite.ErrInvalidRequest)
|
||||
|
||||
// The resumed request grants exactly the approved scopes
|
||||
requester, authorization, err := authorize("openid", "email", "users.read")
|
||||
require.NoError(t, err)
|
||||
require.False(t, authorization.RequiresInteraction)
|
||||
require.ElementsMatch(t, []string{"openid", "email", "users.read"}, []string(requester.GetGrantedScopes()))
|
||||
}
|
||||
|
||||
func TestAuthorizationServiceConsentStepRejectsUnrequestedScopes(t *testing.T) {
|
||||
db := testutils.NewDatabaseForTest(t)
|
||||
service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, "", nil, appconfig.StaticResolver{}), nil, nil, nil)
|
||||
|
||||
const (
|
||||
userID = "test-user"
|
||||
clientID = "test-client"
|
||||
interactionID = "test-interaction"
|
||||
)
|
||||
|
||||
require.NoError(t, db.Create(&model.User{Base: model.Base{ID: userID}}).Error)
|
||||
require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: clientID}, Name: "Test Client"}).Error)
|
||||
require.NoError(t, db.Create(&InteractionSession{
|
||||
Base: model.Base{ID: interactionID},
|
||||
Scopes: datatype.StringList{"openid", "email"},
|
||||
ClientID: clientID,
|
||||
ConsentRequired: true,
|
||||
RequestedAt: datatype.DateTime(time.Now().UTC()),
|
||||
Parameters: map[string]string{"scope": "openid email"},
|
||||
}).Error)
|
||||
|
||||
// The approval may only remove scopes, so a scope the client never requested is rejected
|
||||
_, err := service.completeInteractionStep(t.Context(), interactionID, userID, interactionStepConsent, []string{"email", "groups"}, "", time.Now().UTC(), requestMeta{})
|
||||
require.Error(t, err)
|
||||
|
||||
// Nothing is consented to and the interaction is left untouched
|
||||
var interactionSession InteractionSession
|
||||
require.NoError(t, db.First(&interactionSession, "id = ?", interactionID).Error)
|
||||
require.True(t, interactionSession.ConsentRequired)
|
||||
require.Equal(t, datatype.StringList{"openid", "email"}, interactionSession.Scopes)
|
||||
require.ErrorIs(t, db.First(&model.UserAuthorizedOidcClient{}, "user_id = ? AND client_id = ?", userID, clientID).Error, gorm.ErrRecordNotFound)
|
||||
}
|
||||
|
||||
func TestNarrowInteractionScopes(t *testing.T) {
|
||||
newSession := func() *InteractionSession {
|
||||
return &InteractionSession{
|
||||
Scopes: datatype.StringList{"openid", "profile", "email", "offline_access"},
|
||||
Parameters: map[string]string{"scope": "openid profile email offline_access"},
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("nil approval keeps every requested scope", func(t *testing.T) {
|
||||
session := newSession()
|
||||
require.NoError(t, narrowInteractionScopes(session, nil))
|
||||
require.Equal(t, datatype.StringList{"openid", "profile", "email", "offline_access"}, session.Scopes)
|
||||
require.Equal(t, "openid profile email offline_access", session.Parameters["scope"])
|
||||
})
|
||||
|
||||
t.Run("empty approval keeps only the required scopes", func(t *testing.T) {
|
||||
session := newSession()
|
||||
require.NoError(t, narrowInteractionScopes(session, []string{}))
|
||||
require.Equal(t, datatype.StringList{"openid"}, session.Scopes)
|
||||
require.Equal(t, "openid", session.Parameters["scope"])
|
||||
})
|
||||
|
||||
t.Run("approval keeps the requested order", func(t *testing.T) {
|
||||
session := newSession()
|
||||
require.NoError(t, narrowInteractionScopes(session, []string{"offline_access", "profile"}))
|
||||
require.Equal(t, datatype.StringList{"openid", "profile", "offline_access"}, session.Scopes)
|
||||
require.Equal(t, "openid profile offline_access", session.Parameters["scope"])
|
||||
})
|
||||
|
||||
t.Run("approval of an unrequested scope is rejected", func(t *testing.T) {
|
||||
session := newSession()
|
||||
require.Error(t, narrowInteractionScopes(session, []string{"groups"}))
|
||||
require.Equal(t, datatype.StringList{"openid", "profile", "email", "offline_access"}, session.Scopes)
|
||||
})
|
||||
|
||||
t.Run("nil parameters are initialized", func(t *testing.T) {
|
||||
session := &InteractionSession{Scopes: datatype.StringList{"openid", "email"}}
|
||||
require.NoError(t, narrowInteractionScopes(session, []string{}))
|
||||
require.Equal(t, "openid", session.Parameters["scope"])
|
||||
})
|
||||
}
|
||||
|
||||
func TestAuthorizationServiceConsentMergesAudienceQualifiedScopeKeys(t *testing.T) {
|
||||
db := testutils.NewDatabaseForTest(t)
|
||||
auditLogger := &fakeAuditLogger{}
|
||||
@@ -382,7 +523,7 @@ func TestInteractionAPIClassifiesMissingSession(t *testing.T) {
|
||||
_, err := service.getInteractionSession(t.Context(), "missing-interaction")
|
||||
require.True(t, apperror.IsCode(err, apperror.CodeNotFound))
|
||||
|
||||
_, err = service.completeInteractionStep(t.Context(), "missing-interaction", "user", interactionStepConsent, "", time.Now().UTC(), requestMeta{})
|
||||
_, err = service.completeInteractionStep(t.Context(), "missing-interaction", "user", interactionStepConsent, nil, "", time.Now().UTC(), requestMeta{})
|
||||
require.True(t, apperror.IsCode(err, apperror.CodeNotFound))
|
||||
}
|
||||
|
||||
@@ -599,7 +740,7 @@ func TestAuthorizationServiceCompleteInteractionBindsUserToSession(t *testing.T)
|
||||
Parameters: map[string]string{},
|
||||
}).Error)
|
||||
|
||||
response, err := service.completeInteractionStep(t.Context(), interactionID, userID, interactionStepConsent, "", time.Now().UTC(), requestMeta{})
|
||||
response, err := service.completeInteractionStep(t.Context(), interactionID, userID, interactionStepConsent, nil, "", time.Now().UTC(), requestMeta{})
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, response.RedirectURL)
|
||||
|
||||
@@ -644,7 +785,7 @@ func TestAuthorizationServiceCompleteInteractionSwitchesUserAndResetsRequirement
|
||||
},
|
||||
}).Error)
|
||||
|
||||
response, err := service.completeInteractionStep(t.Context(), interactionID, otherUserID, interactionStepSelectAccount, "", time.Now().UTC(), requestMeta{})
|
||||
response, err := service.completeInteractionStep(t.Context(), interactionID, otherUserID, interactionStepSelectAccount, nil, "", time.Now().UTC(), requestMeta{})
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, response.RedirectURL)
|
||||
require.NotNil(t, response.Interaction)
|
||||
@@ -692,7 +833,7 @@ func TestAuthorizationServiceSelectAccountRecomputesConsentForSelectedUser(t *te
|
||||
require.NoError(t, err)
|
||||
require.True(t, authorization.RequiresInteraction)
|
||||
|
||||
response, err := service.completeInteractionStep(t.Context(), authorization.InteractionID, switchedID, interactionStepSelectAccount, "", time.Now().UTC(), requestMeta{})
|
||||
response, err := service.completeInteractionStep(t.Context(), authorization.InteractionID, switchedID, interactionStepSelectAccount, nil, "", time.Now().UTC(), requestMeta{})
|
||||
require.NoError(t, err)
|
||||
|
||||
// The flow must not be granted yet: consent is still pending for the switched-to user.
|
||||
|
||||
@@ -9,9 +9,16 @@ import (
|
||||
"github.com/pocket-id/fosite"
|
||||
"github.com/pocket-id/pocket-id/backend/internal/apperror"
|
||||
"github.com/pocket-id/pocket-id/backend/internal/authz"
|
||||
"github.com/pocket-id/pocket-id/backend/internal/httpserver"
|
||||
"github.com/pocket-id/pocket-id/backend/internal/utils/cookie"
|
||||
)
|
||||
|
||||
type verifyDeviceCodeRequest struct {
|
||||
// Scopes are the scopes the user approved on the consent screen
|
||||
// Omitting them approves every requested scope
|
||||
Scopes []string `json:"scopes"`
|
||||
}
|
||||
|
||||
type deviceHandler struct {
|
||||
provider fosite.OAuth2Provider
|
||||
deviceService *deviceService
|
||||
@@ -48,12 +55,20 @@ func (h *deviceHandler) verifyDeviceCode(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// The body is optional so callers that send no scope selection approve every requested scope
|
||||
var request verifyDeviceCodeRequest
|
||||
if err := httpserver.BindOptionalJSON(c, &request); err != nil {
|
||||
_ = c.Error(err)
|
||||
return
|
||||
}
|
||||
|
||||
err := h.deviceService.acceptDeviceCode(
|
||||
c.Request.Context(),
|
||||
userCode,
|
||||
principal.UserID,
|
||||
principal.AuthenticationMethod,
|
||||
principal.AuthenticationTime,
|
||||
request.Scopes,
|
||||
reauthenticationToken,
|
||||
requestMetaFromGin(c),
|
||||
)
|
||||
|
||||
@@ -18,7 +18,7 @@ func TestDeviceCodeExchangeConsumesOpenIDConnectSession(t *testing.T) {
|
||||
clientID = "test-client"
|
||||
)
|
||||
service, store, provider, userCode, deviceCode := newTestDeviceServiceWithCode(t, clientID, userID, false, nil)
|
||||
require.NoError(t, service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().UTC(), "", requestMeta{}))
|
||||
require.NoError(t, service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().UTC(), nil, "", requestMeta{}))
|
||||
|
||||
signature, err := provider.deviceStrategy.DeviceCodeSignature(t.Context(), deviceCode)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"slices"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -85,7 +86,7 @@ func (s *deviceService) createDeviceAuthorization(ctx context.Context, req *http
|
||||
}, request, nil
|
||||
}
|
||||
|
||||
func (s *deviceService) acceptDeviceCode(ctx context.Context, userCode, userID, authenticationMethod string, authenticationTime time.Time, reauthenticationToken string, meta requestMeta) error {
|
||||
func (s *deviceService) acceptDeviceCode(ctx context.Context, userCode, userID, authenticationMethod string, authenticationTime time.Time, approvedScopes []string, reauthenticationToken string, meta requestMeta) error {
|
||||
request, userCodeSignature, err := s.deviceRequestFromUserCode(ctx, userCode)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -107,6 +108,11 @@ func (s *deviceService) acceptDeviceCode(ctx context.Context, userCode, userID,
|
||||
return fosite.ErrAccessDenied.WithHint("You are not allowed to access this service.")
|
||||
}
|
||||
|
||||
// Narrow the request to the scopes the user approved before resolving the grant, so deselected scopes are neither consented to nor granted
|
||||
if err := narrowDeviceRequestScopes(request, approvedScopes); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
resource, err := request.GetResource()
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -169,6 +175,31 @@ func (s *deviceService) acceptDeviceCode(ctx context.Context, userCode, userID,
|
||||
})
|
||||
}
|
||||
|
||||
// narrowDeviceRequestScopes restricts a pending device authorization to the scopes the user approved on the consent screen
|
||||
// The device authorization endpoint already granted every allowed scope, so the grants of deselected scopes are dropped as well
|
||||
func narrowDeviceRequestScopes(request fosite.DeviceRequester, approvedScopes []string) error {
|
||||
if approvedScopes == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
narrowed, err := narrowApprovedScopes(request.GetRequestedScopes(), approvedScopes)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// The requester interface can only add grants, so the stored device request is changed directly
|
||||
deviceRequest, ok := request.(*fosite.DeviceRequest)
|
||||
if !ok {
|
||||
return errors.New("unexpected device request type")
|
||||
}
|
||||
deviceRequest.SetRequestedScopes(narrowed)
|
||||
deviceRequest.GrantedScope = slices.DeleteFunc(deviceRequest.GrantedScope, func(scope string) bool {
|
||||
return !slices.Contains(narrowed, scope)
|
||||
})
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// copyDeviceCodeExpiry copies the device and user code expiry from the pending device authorization's session to the session that replaces it
|
||||
func copyDeviceCodeExpiry(from fosite.Session, to *Session) {
|
||||
if from == nil {
|
||||
|
||||
@@ -52,7 +52,7 @@ func TestDeviceServiceAcceptRequiresReauthenticationTokenWhenClientRequiresIt(t
|
||||
}
|
||||
service, _, _, userCode, _ := newTestDeviceServiceWithCode(t, clientID, userID, true, reauth)
|
||||
|
||||
err := service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().UTC(), "", requestMeta{})
|
||||
err := service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().UTC(), nil, "", requestMeta{})
|
||||
require.True(t, apperror.IsCode(err, apperror.CodeReauthenticationRequired))
|
||||
require.Zero(t, reauth.calls)
|
||||
|
||||
@@ -60,7 +60,7 @@ func TestDeviceServiceAcceptRequiresReauthenticationTokenWhenClientRequiresIt(t
|
||||
require.NoError(t, err)
|
||||
require.True(t, info.ReauthenticationRequired)
|
||||
|
||||
err = service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().UTC(), reauth.token, requestMeta{})
|
||||
err = service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().UTC(), nil, reauth.token, requestMeta{})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 1, reauth.calls)
|
||||
}
|
||||
@@ -86,7 +86,7 @@ func TestDeviceServiceAcceptUsesReauthenticationTimeForDeviceSession(t *testing.
|
||||
}
|
||||
service, store, provider, userCode, deviceCode := newTestDeviceServiceWithCode(t, clientID, userID, true, reauth)
|
||||
|
||||
err := service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().Add(-time.Hour).UTC(), reauth.token, requestMeta{})
|
||||
err := service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().Add(-time.Hour).UTC(), nil, reauth.token, requestMeta{})
|
||||
require.NoError(t, err)
|
||||
|
||||
deviceCodeSignature, err := provider.deviceStrategy.DeviceCodeSignature(t.Context(), deviceCode)
|
||||
@@ -97,6 +97,46 @@ func TestDeviceServiceAcceptUsesReauthenticationTimeForDeviceSession(t *testing.
|
||||
require.Equal(t, reauthenticatedAt, session.IDTokenClaims().AuthTime)
|
||||
}
|
||||
|
||||
// TestDeviceServiceAcceptGrantsOnlyApprovedScopes guards that scopes the user deselects on the device consent screen are neither consented to nor granted, even though the device authorization endpoint granted them up front
|
||||
func TestDeviceServiceAcceptGrantsOnlyApprovedScopes(t *testing.T) {
|
||||
const (
|
||||
userID = "test-user"
|
||||
clientID = "test-client"
|
||||
)
|
||||
service, store, provider := newTestDeviceService(t, clientID, userID, false, nil, nil)
|
||||
userCode, deviceCode := createTestDeviceCode(t, service, clientID, "openid email groups")
|
||||
|
||||
// The user deselects email and does not send openid, which must still be kept
|
||||
require.NoError(t, service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().UTC(), []string{"groups"}, "", requestMeta{}))
|
||||
|
||||
// Consent is only recorded for the approved scopes
|
||||
var authorizedClient model.UserAuthorizedOidcClient
|
||||
require.NoError(t, store.db.First(&authorizedClient, "user_id = ? AND client_id = ?", userID, clientID).Error)
|
||||
require.ElementsMatch(t, []string{"openid", "groups"}, []string(authorizedClient.Scope))
|
||||
|
||||
// The token the device receives only carries the approved scopes
|
||||
response, err := exchangeDeviceCode(t, provider, clientID, deviceCode)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "openid groups", response.GetExtra("scope"))
|
||||
}
|
||||
|
||||
func TestDeviceServiceAcceptRejectsUnrequestedScopes(t *testing.T) {
|
||||
const (
|
||||
userID = "test-user"
|
||||
clientID = "test-client"
|
||||
)
|
||||
service, store, _ := newTestDeviceService(t, clientID, userID, false, nil, nil)
|
||||
userCode, _ := createTestDeviceCode(t, service, clientID, "openid email")
|
||||
|
||||
// The approval may only remove scopes, so a scope the device never requested is rejected
|
||||
err := service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().UTC(), []string{"email", "groups"}, "", requestMeta{})
|
||||
require.Error(t, err)
|
||||
|
||||
// Nothing is consented to and the user code can still be approved
|
||||
require.ErrorIs(t, store.db.First(&model.UserAuthorizedOidcClient{}, "user_id = ? AND client_id = ?", userID, clientID).Error, gorm.ErrRecordNotFound)
|
||||
require.NoError(t, service.acceptDeviceCode(t.Context(), userCode, userID, "phr", time.Now().UTC(), nil, "", requestMeta{}))
|
||||
}
|
||||
|
||||
func TestDeviceServiceCreateRejectsResourceWithoutUserDelegatedGrant(t *testing.T) {
|
||||
const (
|
||||
userID = "test-user"
|
||||
@@ -129,17 +169,25 @@ func newTestDeviceServiceWithCode(t *testing.T, clientID, userID string, require
|
||||
t.Helper()
|
||||
|
||||
service, store, provider := newTestDeviceService(t, clientID, userID, requiresReauthentication, reauth, nil)
|
||||
userCode, deviceCode := createTestDeviceCode(t, service, clientID, "openid")
|
||||
|
||||
return service, store, provider, userCode, deviceCode
|
||||
}
|
||||
|
||||
// createTestDeviceCode starts a device authorization for the given scope like a device would
|
||||
func createTestDeviceCode(t *testing.T, service *deviceService, clientID, scope string) (userCode string, deviceCode string) {
|
||||
t.Helper()
|
||||
|
||||
form := url.Values{
|
||||
"client_id": {clientID},
|
||||
"scope": {"openid"},
|
||||
"scope": {scope},
|
||||
}
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/api/oidc/device/authorize", strings.NewReader(form.Encode()))
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
response, _, err := service.createDeviceAuthorization(t.Context(), req)
|
||||
require.NoError(t, err)
|
||||
|
||||
return service, store, provider, response.UserCode, response.DeviceCode
|
||||
return response.UserCode, response.DeviceCode
|
||||
}
|
||||
|
||||
func newTestDeviceService(t *testing.T, clientID, userID string, requiresReauthentication bool, reauth ReauthenticationTokenConsumer, apiAccess APIAccessProvider) (*deviceService, *Store, *oidcProvider) {
|
||||
|
||||
@@ -32,6 +32,9 @@ type interactionClient struct {
|
||||
|
||||
type completeInteractionRequest struct {
|
||||
Step interactionStep `json:"step"`
|
||||
// Scopes are the scopes the user approved on the consent screen
|
||||
// Omitting them approves every requested scope
|
||||
Scopes []string `json:"scopes"`
|
||||
}
|
||||
|
||||
type completeInteractionResponse struct {
|
||||
|
||||
@@ -59,6 +59,7 @@ func (s *interactionSessionService) update(ctx context.Context, interactionSessi
|
||||
"reauthentication_required": interactionSession.ReauthenticationRequired,
|
||||
"consent_required": interactionSession.ConsentRequired,
|
||||
"user_id": interactionSession.UserID,
|
||||
"scopes": interactionSession.Scopes,
|
||||
"reauthenticated_at": interactionSession.ReauthenticatedAt,
|
||||
"parameters": interactionSession.Parameters,
|
||||
}).
|
||||
|
||||
@@ -1,19 +1,23 @@
|
||||
<script lang="ts">
|
||||
import Checkbox from '#lib/components/ui/checkbox/checkbox.svelte';
|
||||
import * as Item from '#lib/components/ui/item/index.js';
|
||||
import { cn } from '#lib/utils/style.js';
|
||||
import type { LucideIcon } from '@lucide/svelte';
|
||||
|
||||
interface Props {
|
||||
icon: LucideIcon;
|
||||
name: string;
|
||||
description: string;
|
||||
selected?: boolean;
|
||||
onSelectedChange?: (selected: boolean) => void;
|
||||
}
|
||||
|
||||
let { icon, name, description }: Props = $props();
|
||||
let { icon, name, description, selected = true, onSelectedChange }: Props = $props();
|
||||
|
||||
const SvelteComponent = $derived(icon);
|
||||
</script>
|
||||
|
||||
<Item.Root class="px-0 py-1.5">
|
||||
{#snippet content()}
|
||||
<Item.Media class="bg-muted h-full !translate-y-0 !self-center rounded-lg p-2">
|
||||
<SvelteComponent class="size-5" />
|
||||
</Item.Media>
|
||||
@@ -21,4 +25,30 @@
|
||||
<Item.Title class="font-semibold">{name}</Item.Title>
|
||||
<Item.Description>{description}</Item.Description>
|
||||
</Item.Content>
|
||||
</Item.Root>
|
||||
{/snippet}
|
||||
|
||||
{#if onSelectedChange}
|
||||
<!-- The whole row is a label so clicking anywhere on it toggles the scope -->
|
||||
<Item.Root class="px-0 py-1.5">
|
||||
{#snippet child({ props })}
|
||||
<label
|
||||
{...props}
|
||||
class={cn(props.class as string, 'cursor-pointer', !selected && 'opacity-60')}
|
||||
data-testid="scope-item"
|
||||
>
|
||||
{@render content()}
|
||||
<Item.Actions>
|
||||
<Checkbox
|
||||
aria-label={name}
|
||||
checked={selected}
|
||||
onCheckedChange={(checked: boolean) => onSelectedChange(checked)}
|
||||
/>
|
||||
</Item.Actions>
|
||||
</label>
|
||||
{/snippet}
|
||||
</Item.Root>
|
||||
{:else}
|
||||
<Item.Root class="px-0 py-1.5">
|
||||
{@render content()}
|
||||
</Item.Root>
|
||||
{/if}
|
||||
|
||||
@@ -7,23 +7,49 @@
|
||||
|
||||
let {
|
||||
scopes,
|
||||
scopeInfo = []
|
||||
}: { scopes?: string[] | null; scopeInfo?: InteractionScopeInfo[] | null } = $props();
|
||||
scopeInfo = [],
|
||||
selectedScopes = $bindable()
|
||||
}: {
|
||||
scopes?: string[] | null;
|
||||
scopeInfo?: InteractionScopeInfo[] | null;
|
||||
// When bound, every displayed scope gets a checkbox so the user can deselect it
|
||||
selectedScopes?: string[];
|
||||
} = $props();
|
||||
|
||||
const standardScopes = ['openid', 'profile', 'email', 'groups', 'offline_access'];
|
||||
const infoByKey = $derived(new Map((scopeInfo || []).map((info) => [info.key, info])));
|
||||
const customScopes = $derived((scopes || []).filter((scope) => !standardScopes.includes(scope)));
|
||||
|
||||
function selectionProps(scope: string) {
|
||||
if (!selectedScopes) {
|
||||
return {};
|
||||
}
|
||||
|
||||
return {
|
||||
selected: selectedScopes.includes(scope),
|
||||
onSelectedChange: (selected: boolean) => {
|
||||
const others = (selectedScopes ?? []).filter((s) => s !== scope);
|
||||
selectedScopes = selected ? [...others, scope] : others;
|
||||
}
|
||||
};
|
||||
}
|
||||
</script>
|
||||
|
||||
<Item.Group data-testid="scopes" class="gap-1">
|
||||
{#if (scopes || []).includes('email')}
|
||||
<ScopeItem icon={LucideMail} name={m.email()} description={m.view_your_email_address()} />
|
||||
<ScopeItem
|
||||
icon={LucideMail}
|
||||
name={m.email()}
|
||||
description={m.view_your_email_address()}
|
||||
{...selectionProps('email')}
|
||||
/>
|
||||
{/if}
|
||||
{#if (scopes || []).includes('profile')}
|
||||
<ScopeItem
|
||||
icon={LucideUser}
|
||||
name={m.profile()}
|
||||
description={m.view_your_profile_information()}
|
||||
{...selectionProps('profile')}
|
||||
/>
|
||||
{/if}
|
||||
{#if (scopes || []).includes('groups')}
|
||||
@@ -31,6 +57,7 @@
|
||||
icon={LucideUsers}
|
||||
name={m.groups()}
|
||||
description={m.view_the_groups_you_are_a_member_of()}
|
||||
{...selectionProps('groups')}
|
||||
/>
|
||||
{/if}
|
||||
{#each customScopes as scope (scope)}
|
||||
@@ -38,6 +65,7 @@
|
||||
icon={LucideKeyRound}
|
||||
name={infoByKey.get(scope)?.name ?? scope}
|
||||
description={infoByKey.get(scope)?.description || m.access_an_api_on_your_behalf()}
|
||||
{...selectionProps(scope)}
|
||||
/>
|
||||
{/each}
|
||||
</Item.Group>
|
||||
|
||||
@@ -27,10 +27,14 @@ class OidcService extends APIService {
|
||||
return data;
|
||||
};
|
||||
|
||||
completeAuthorizeInteractionStep = async (id: string, step: InteractionStep) => {
|
||||
completeAuthorizeInteractionStep = async (
|
||||
id: string,
|
||||
step: InteractionStep,
|
||||
scopes?: string[]
|
||||
) => {
|
||||
const { data } = await this.api.post<CompleteInteractionResponse>(
|
||||
`/oidc/interactions/${id}/complete`,
|
||||
{ step }
|
||||
{ step, scopes }
|
||||
);
|
||||
return data;
|
||||
};
|
||||
@@ -116,8 +120,8 @@ class OidcService extends APIService {
|
||||
return res.data as OidcClientWithAllowedUserGroups;
|
||||
};
|
||||
|
||||
verifyDeviceCode = async (userCode: string) => {
|
||||
return await this.api.post(`/oidc/device/verify?code=${userCode}`);
|
||||
verifyDeviceCode = async (userCode: string, scopes?: string[]) => {
|
||||
return await this.api.post(`/oidc/device/verify?code=${userCode}`, { scopes });
|
||||
};
|
||||
|
||||
getDeviceCodeInfo = async (userCode: string): Promise<OidcDeviceCodeInfo> => {
|
||||
|
||||
@@ -37,6 +37,7 @@
|
||||
let deviceLoginDecision: 'approve' | 'deny' | undefined = $state();
|
||||
let errorMessage: string | null = $state(null);
|
||||
let authorizationRequired = $state(false);
|
||||
let selectedScopes: string[] = $state([]);
|
||||
let reauthenticationRequired = $state(false);
|
||||
let reauthenticated = $state(false);
|
||||
let normalizedUserCode = $derived(
|
||||
@@ -79,6 +80,8 @@
|
||||
|
||||
if (info.authorizationRequired && !authorizationRequired) {
|
||||
authorizationRequired = true;
|
||||
// Every requested scope starts selected so the user only has to deselect the ones they do not want to grant
|
||||
selectedScopes = [...(info.scope ?? [])];
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -92,7 +95,10 @@
|
||||
reauthenticated = true;
|
||||
}
|
||||
|
||||
await oidcService.verifyDeviceCode(normalizedUserCode);
|
||||
await oidcService.verifyDeviceCode(
|
||||
normalizedUserCode,
|
||||
authorizationRequired ? selectedScopes : undefined
|
||||
);
|
||||
success = true;
|
||||
} catch (error) {
|
||||
errorMessage = getWebauthnErrorMessage(error);
|
||||
@@ -236,7 +242,11 @@
|
||||
</Card.Description>
|
||||
</Card.Header>
|
||||
<Card.Content data-testid="scopes">
|
||||
<ScopeList scopes={deviceInfo!.scope || []} scopeInfo={deviceInfo!.scopeInfo || []} />
|
||||
<ScopeList
|
||||
scopes={deviceInfo!.scope || []}
|
||||
scopeInfo={deviceInfo!.scopeInfo || []}
|
||||
bind:selectedScopes
|
||||
/>
|
||||
</Card.Content>
|
||||
</Card.Root>
|
||||
</div>
|
||||
|
||||
@@ -30,6 +30,8 @@
|
||||
let errorMessage: string | null = $state(null);
|
||||
let errorCode: string | undefined = $state();
|
||||
let currentStep = $derived(interactionSession.currentStep);
|
||||
// Every requested scope starts selected so the user only has to deselect the ones they do not want to grant
|
||||
let selectedScopes = $state([...(data.interactionSession.scopes ?? [])]);
|
||||
|
||||
const fullName = $derived.by(() => {
|
||||
if (!$userStore) {
|
||||
@@ -93,7 +95,11 @@
|
||||
}
|
||||
|
||||
async function completeInteraction(step: InteractionStep, skipRedirect = false) {
|
||||
const result = await oidcService.completeAuthorizeInteractionStep(interactionSession.id, step);
|
||||
const result = await oidcService.completeAuthorizeInteractionStep(
|
||||
interactionSession.id,
|
||||
step,
|
||||
step === 'consent' ? selectedScopes : undefined
|
||||
);
|
||||
if (result.interaction) {
|
||||
interactionSession = result.interaction;
|
||||
if (interactionSession.currentStep == 'reauthenticate') {
|
||||
@@ -193,6 +199,7 @@
|
||||
<ScopeList
|
||||
scopes={interactionSession.scopes || []}
|
||||
scopeInfo={interactionSession.scopeInfo ?? []}
|
||||
bind:selectedScopes
|
||||
/>
|
||||
</Card.Content>
|
||||
</Card.Root>
|
||||
|
||||
@@ -120,6 +120,38 @@ test('Authorize client requesting offline_access scope', async ({ page }) => {
|
||||
expect(callbackUrl.searchParams.get('error')).toBeNull();
|
||||
});
|
||||
|
||||
test('Authorize new client with a deselected scope', async ({ page }) => {
|
||||
const oidcClient = oidcClients.immich;
|
||||
const urlParams = createUrlParams(oidcClient);
|
||||
await page.goto(`/authorize?${urlParams.toString()}`);
|
||||
|
||||
await expectScopes(page, ['Email', 'Profile']);
|
||||
await page.getByRole('checkbox', { name: 'Email' }).click();
|
||||
await expect(page.getByRole('checkbox', { name: 'Email' })).not.toBeChecked();
|
||||
await expect(page.getByRole('checkbox', { name: 'Profile' })).toBeChecked();
|
||||
|
||||
const callbackUrl = await expectCallbackRedirect(page, oidcClient.callbackUrl, () =>
|
||||
page.getByRole('button', { name: 'Sign in' }).click()
|
||||
);
|
||||
|
||||
// The deselected scope must be missing from the issued token and therefore from the userinfo claims
|
||||
const token = (await oidcUtil.exchangeCode(page, {
|
||||
grant_type: 'authorization_code',
|
||||
code: callbackUrl.searchParams.get('code')!,
|
||||
redirect_uri: oidcClient.callbackUrl,
|
||||
client_id: oidcClient.id,
|
||||
client_secret: oidcClient.secret
|
||||
})) as { access_token?: string; scope?: string };
|
||||
expect(token.scope?.split(' ').sort()).toEqual(['openid', 'profile']);
|
||||
|
||||
const userinfo = await page.request
|
||||
.get('/api/oidc/userinfo', { headers: { Authorization: `Bearer ${token.access_token}` } })
|
||||
.then((r) => r.json());
|
||||
expect(userinfo.sub).toBe(users.tim.id);
|
||||
expect(userinfo.given_name).toBeTruthy();
|
||||
expect(userinfo.email).toBeUndefined();
|
||||
});
|
||||
|
||||
test('Authorize existing client without scopes', async ({ page }) => {
|
||||
const oidcClient = oidcClients.nextcloud;
|
||||
const urlParams = new URLSearchParams({
|
||||
@@ -678,6 +710,47 @@ test('Authorize new client with device authorization flow', async ({ page }) =>
|
||||
).toBeVisible();
|
||||
});
|
||||
|
||||
test('Authorize new client with device authorization flow and a deselected scope', async ({
|
||||
page
|
||||
}) => {
|
||||
const client = oidcClients.immich;
|
||||
const credentials = { client_id: client.id, client_secret: client.secret };
|
||||
const device = await page.request
|
||||
.post('/api/oidc/device/authorize', {
|
||||
form: { ...credentials, scope: 'openid profile email' }
|
||||
})
|
||||
.then((r) => r.json());
|
||||
|
||||
await page.goto(`/device?user_code=${device.user_code}`);
|
||||
await page.getByRole('button', { name: 'Authorize' }).click();
|
||||
|
||||
await expectScopes(page, ['Email', 'Profile']);
|
||||
await page.getByRole('checkbox', { name: 'Email' }).click();
|
||||
await expect(page.getByRole('checkbox', { name: 'Email' })).not.toBeChecked();
|
||||
await expect(page.getByRole('checkbox', { name: 'Profile' })).toBeChecked();
|
||||
|
||||
await page.getByRole('button', { name: 'Authorize' }).click();
|
||||
await expect(
|
||||
page.getByRole('paragraph').filter({ hasText: 'The device has been authorized.' })
|
||||
).toBeVisible();
|
||||
|
||||
// The deselected scope must be missing from the token the device receives and therefore from the userinfo claims
|
||||
await page.waitForTimeout(device.interval * 1000 + 100);
|
||||
const token = (await oidcUtil.exchangeCode(page, {
|
||||
...credentials,
|
||||
grant_type: 'urn:ietf:params:oauth:grant-type:device_code',
|
||||
device_code: device.device_code
|
||||
})) as { access_token?: string; scope?: string };
|
||||
expect(token.scope?.split(' ').sort()).toEqual(['openid', 'profile']);
|
||||
|
||||
const userinfo = await page.request
|
||||
.get('/api/oidc/userinfo', { headers: { Authorization: `Bearer ${token.access_token}` } })
|
||||
.then((r) => r.json());
|
||||
expect(userinfo.sub).toBe(users.tim.id);
|
||||
expect(userinfo.given_name).toBeTruthy();
|
||||
expect(userinfo.email).toBeUndefined();
|
||||
});
|
||||
|
||||
test('Authorize new client with device authorization flow while not signed in', async ({
|
||||
page
|
||||
}) => {
|
||||
|
||||
Reference in New Issue
Block a user