Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion pkg/authserver/server/handlers/callback.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,15 @@ func (h *Handler) CallbackHandler(w http.ResponseWriter, req *http.Request) {
user, err := h.userResolver.ResolveUser(ctx, providerID, providerSubject)
if err != nil {
slog.Error("failed to resolve user", "error", err)
h.provider.WriteAuthorizeError(ctx, w, ar, fosite.ErrServerError.WithHint("failed to resolve user"))
// A UserStorage that deliberately refuses to auto-provision this
// identity (e.g. SCIM-only provisioning) is a client-facing denial,
// not a server fault — every other resolution failure keeps the
// generic server_error mapping.
resolveErr := fosite.ErrServerError.WithHint("failed to resolve user")
if errors.Is(err, storage.ErrUserNotProvisioned) {
resolveErr = fosite.ErrAccessDenied.WithHint("user not provisioned")
}
h.provider.WriteAuthorizeError(ctx, w, ar, resolveErr)
return
}
subject = user.ID
Expand Down
78 changes: 78 additions & 0 deletions pkg/authserver/server/handlers/callback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,84 @@ func TestCallbackHandler_IdentityResolutionFailure(t *testing.T) {

// --- Multi-upstream chain tests ---

func TestCallbackHandler_UserResolutionFailure_UserNotProvisioned_DeniesAccess(t *testing.T) {
t.Parallel()
handler, storState, mockUpstream := handlerTestSetup(t, withCreateUserError(storage.ErrUserNotProvisioned))

// A subject with no existing provider identity takes UserResolver's create
// path, where the configured storage refuses to auto-provision it.
mockUpstream.exchangeResult = &upstream.Identity{
Tokens: &upstream.Tokens{
AccessToken: "upstream-access-token",
ExpiresAt: time.Now().Add(time.Hour),
},
Subject: "unprovisioned-user",
}

internalState := testInternalState
pending := &storage.PendingAuthorization{
ClientID: testAuthClientID,
RedirectURI: testAuthRedirectURI,
State: "client-state",
PKCEChallenge: "challenge123",
PKCEMethod: "S256",
Scopes: []string{"openid"},
InternalState: internalState,
SessionID: "session-not-provisioned",
UpstreamProviderName: "test-upstream",
CreatedAt: time.Now(),
}
storState.pendingAuths[internalState] = pending

req := httptest.NewRequest(http.MethodGet, "/oauth/callback?code=upstream-code&state="+internalState, nil)
rec := httptest.NewRecorder()

handler.CallbackHandler(rec, req)

assert.Equal(t, http.StatusSeeOther, rec.Code)
location := rec.Header().Get("Location")
assert.Contains(t, location, "error=access_denied",
"a deliberate provisioning refusal must deny the login, not surface as a server error")
}

func TestCallbackHandler_UserResolutionFailure_OtherError_ServerError(t *testing.T) {
t.Parallel()
handler, storState, mockUpstream := handlerTestSetup(t, withCreateUserError(assert.AnError))

mockUpstream.exchangeResult = &upstream.Identity{
Tokens: &upstream.Tokens{
AccessToken: "upstream-access-token",
ExpiresAt: time.Now().Add(time.Hour),
},
Subject: "some-user",
}

internalState := testInternalState
pending := &storage.PendingAuthorization{
ClientID: testAuthClientID,
RedirectURI: testAuthRedirectURI,
State: "client-state",
PKCEChallenge: "challenge123",
PKCEMethod: "S256",
Scopes: []string{"openid"},
InternalState: internalState,
SessionID: "session-other-error",
UpstreamProviderName: "test-upstream",
CreatedAt: time.Now(),
}
storState.pendingAuths[internalState] = pending

req := httptest.NewRequest(http.MethodGet, "/oauth/callback?code=upstream-code&state="+internalState, nil)
rec := httptest.NewRecorder()

handler.CallbackHandler(rec, req)

assert.Equal(t, http.StatusSeeOther, rec.Code)
location := rec.Header().Get("Location")
assert.Contains(t, location, "error=server_error",
"an internal storage failure unrelated to provisioning refusal keeps mapping to server_error")
}

func TestCallbackHandler_TwoUpstreams_FirstLeg_RedirectsToSecond(t *testing.T) {
t.Parallel()
handler, storState, provider1, _ := multiUpstreamTestSetup(t)
Expand Down
22 changes: 17 additions & 5 deletions pkg/authserver/server/handlers/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ type baseTestSetupOption func(*baseTestSetupConfig)
type baseTestSetupConfig struct {
storePendingErr error // if non-nil, StorePendingAuthorization always returns this error
getLatestUpstreamTokensErr error // if non-nil, GetLatestUpstreamTokensForUser always returns this error
createUserErr error // if non-nil, CreateUser always returns this error
}

func withStorePendingError(err error) baseTestSetupOption {
Expand All @@ -121,6 +122,12 @@ func withGetLatestUpstreamTokensError(err error) baseTestSetupOption {
}
}

func withCreateUserError(err error) baseTestSetupOption {
return func(c *baseTestSetupConfig) {
c.createUserErr = err
}
}

// baseTestSetup creates the shared test infrastructure (RSA keys, fosite provider, mock storage
// with all expectations wired, including upstream token mocks). Callers create the Handler.
func baseTestSetup(t *testing.T, opts ...baseTestSetupOption) (fosite.OAuth2Provider, *server.AuthorizationServerConfig, *mocks.MockStorage, *testStorageState) {
Expand Down Expand Up @@ -289,11 +296,16 @@ func baseTestSetup(t *testing.T, opts ...baseTestSetupOption) (fosite.OAuth2Prov
stor.EXPECT().RevokeRefreshToken(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()

// Setup mock expectations for user storage (needed by UserResolver)
stor.EXPECT().CreateUser(gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, user *storage.User) error {
storState.users[user.ID] = user
return nil
}).AnyTimes()
if setupCfg.createUserErr != nil {
// CreateUser always fails with the configured error
stor.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(setupCfg.createUserErr).AnyTimes()
} else {
stor.EXPECT().CreateUser(gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, user *storage.User) error {
storState.users[user.ID] = user
return nil
}).AnyTimes()
}

stor.EXPECT().GetUser(gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, id string) (*storage.User, error) {
Expand Down
12 changes: 12 additions & 0 deletions pkg/authserver/storage/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ var (
// ErrReservedClientID is returned when a caller attempts to register a
// real client whose ID collides with SyntheticClientIDPrefix.
ErrReservedClientID = errors.New("storage: client id uses reserved synthetic prefix")

// ErrUserNotProvisioned is returned by a UserStorage.CreateUser implementation
// that deliberately refuses to auto-provision an upstream identity (e.g. a
// deployment where user accounts are only created out-of-band, such as via
// SCIM). It signals the authorization callback to deny the login with
// fosite.ErrAccessDenied instead of treating the refusal as an internal
// server error.
ErrUserNotProvisioned = errors.New("storage: user not provisioned")
)

// DefaultPendingAuthorizationTTL is the default TTL for pending authorization requests.
Expand Down Expand Up @@ -748,6 +756,10 @@ type UpstreamTokenRefresher interface {
type UserStorage interface {
// CreateUser creates a new user account.
// Returns ErrAlreadyExists if a user with the same ID already exists.
// A deployment that provisions users out-of-band (e.g. via SCIM) and never
// auto-creates them here may return ErrUserNotProvisioned to deny the login
// instead; the caller (the authorization callback) maps that specifically to
// an OAuth access_denied response rather than a server error.
CreateUser(ctx context.Context, user *User) error

// GetUser retrieves a user by their internal ID.
Expand Down
Loading