mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-29 11:01:29 +02:00
[management] Deny reverse proxy access to pending and blocked users
The reverse proxy authorization path only checked the session token, the user's account, and group membership, so a user awaiting approval — stored as blocked and pending approval — could complete SSO and reach any service distributed to a group they belong to, including the All Users group. The REST API and peer registration gate on the same state through the permissions manager, but the proxy gRPC service never consults it. ValidateSession now denies on account status with pending_approval or user_blocked as the reason, so proxy access logs and the denied page carry the cause. GenerateSessionToken refuses to mint a token for such a user at all, which keeps the session cookie out of the browser and lets the OIDC callback tell the user why. ValidateUserGroupAccess and ValidateTunnelPeer close the same gap, the latter for a user blocked after registering a peer. A user the store cannot resolve is denied rather than passed through.
This commit is contained in:
@@ -29,6 +29,7 @@ import (
|
||||
|
||||
"github.com/netbirdio/netbird/shared/management/domain"
|
||||
|
||||
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
|
||||
"github.com/netbirdio/netbird/management/internals/modules/peers"
|
||||
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs"
|
||||
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy"
|
||||
@@ -36,7 +37,6 @@ import (
|
||||
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/sessionkey"
|
||||
"github.com/netbirdio/netbird/management/server/idp"
|
||||
"github.com/netbirdio/netbird/management/server/peer"
|
||||
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
|
||||
"github.com/netbirdio/netbird/management/server/types"
|
||||
"github.com/netbirdio/netbird/management/server/users"
|
||||
proxyauth "github.com/netbirdio/netbird/proxy/auth"
|
||||
@@ -1579,9 +1579,55 @@ func (s *ProxyServiceServer) ValidateState(state string) (verifier, redirectURL
|
||||
return verifier, redirectURL, nil
|
||||
}
|
||||
|
||||
// Denied reasons reported to the proxy when access is refused because of the
|
||||
// account status of the user behind the request.
|
||||
const (
|
||||
deniedReasonPendingApproval = "pending_approval"
|
||||
deniedReasonUserBlocked = "user_blocked"
|
||||
deniedReasonUserNotFound = "user_not_found"
|
||||
)
|
||||
|
||||
var (
|
||||
// ErrUserPendingApproval reports a user whose account still awaits approval
|
||||
// by an administrator and may therefore not hold a proxy session.
|
||||
ErrUserPendingApproval = errors.New("user pending approval")
|
||||
|
||||
// ErrUserBlocked reports a blocked user, who may not hold a proxy session.
|
||||
ErrUserBlocked = errors.New("user blocked")
|
||||
|
||||
errUserUnresolved = errors.New("user could not be resolved")
|
||||
)
|
||||
|
||||
// checkUserStatus reports whether the user's account status permits reverse
|
||||
// proxy access, returning the denied reason for the proxy access log together
|
||||
// with the sentinel error callers match on. A user awaiting approval is stored
|
||||
// as both pending and blocked, so the pending state is reported first: it is
|
||||
// the one an administrator can act on.
|
||||
func checkUserStatus(user *types.User) (string, error) {
|
||||
switch {
|
||||
case user == nil:
|
||||
return deniedReasonUserNotFound, errUserUnresolved
|
||||
case user.PendingApproval:
|
||||
return deniedReasonPendingApproval, ErrUserPendingApproval
|
||||
case user.IsBlocked():
|
||||
return deniedReasonUserBlocked, ErrUserBlocked
|
||||
default:
|
||||
return "", nil
|
||||
}
|
||||
}
|
||||
|
||||
// userStatusDeniedReason returns the denied reason for callers that report a
|
||||
// decision rather than an error, and an empty string when the user may proceed.
|
||||
func userStatusDeniedReason(user *types.User) string {
|
||||
reason, _ := checkUserStatus(user)
|
||||
return reason
|
||||
}
|
||||
|
||||
// GenerateSessionToken creates a signed session JWT for the given domain and
|
||||
// user. The user's group memberships are embedded in the token so policy-aware
|
||||
// middlewares on the proxy can authorise without an extra management round-trip.
|
||||
// A user the store cannot resolve, or whose account is pending approval or
|
||||
// blocked, gets no token at all, so the browser never receives a session cookie.
|
||||
func (s *ProxyServiceServer) GenerateSessionToken(ctx context.Context, domain, userID string, method proxyauth.Method) (string, error) {
|
||||
service, err := s.getServiceByDomain(ctx, domain)
|
||||
if err != nil {
|
||||
@@ -1592,25 +1638,25 @@ func (s *ProxyServiceServer) GenerateSessionToken(ctx context.Context, domain, u
|
||||
return "", fmt.Errorf("no session key configured for domain: %s", domain)
|
||||
}
|
||||
|
||||
var (
|
||||
email string
|
||||
groupIDs []string
|
||||
groupNames []string
|
||||
)
|
||||
if s.usersManager != nil {
|
||||
user, userGroups, uerr := s.usersManager.GetUserWithGroups(ctx, userID)
|
||||
if uerr != nil {
|
||||
log.WithContext(ctx).Debugf("session token mint: lookup user %s: %v", userID, uerr)
|
||||
} else if user != nil {
|
||||
email = user.Email
|
||||
groupIDs, groupNames = pairGroupIDsAndNames(userGroups)
|
||||
}
|
||||
if s.usersManager == nil {
|
||||
return "", errors.New("users manager not configured")
|
||||
}
|
||||
|
||||
user, userGroups, err := s.usersManager.GetUserWithGroups(ctx, userID)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("get user %s: %w", userID, err)
|
||||
}
|
||||
|
||||
if _, err := checkUserStatus(user); err != nil {
|
||||
return "", fmt.Errorf("session token for user %s: %w", userID, err)
|
||||
}
|
||||
|
||||
groupIDs, groupNames := pairGroupIDsAndNames(userGroups)
|
||||
|
||||
return sessionkey.SignToken(
|
||||
service.SessionPrivateKey,
|
||||
userID,
|
||||
email,
|
||||
user.Email,
|
||||
domain,
|
||||
method,
|
||||
groupIDs,
|
||||
@@ -1628,6 +1674,10 @@ func (s *ProxyServiceServer) ValidateUserGroupAccess(ctx context.Context, domain
|
||||
return fmt.Errorf("user not found: %s", userID)
|
||||
}
|
||||
|
||||
if _, err := checkUserStatus(user); err != nil {
|
||||
return fmt.Errorf("user %s denied access to domain %s: %w", userID, domain, err)
|
||||
}
|
||||
|
||||
service, err := s.getAccountServiceByDomain(ctx, user.AccountID, domain)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -1732,16 +1782,16 @@ func (s *ProxyServiceServer) ValidateSession(ctx context.Context, req *proto.Val
|
||||
}
|
||||
|
||||
user, userGroups, err := s.usersManager.GetUserWithGroups(ctx, userID)
|
||||
if err != nil {
|
||||
if err != nil || user == nil {
|
||||
log.WithFields(log.Fields{
|
||||
"domain": domain,
|
||||
"user_id": userID,
|
||||
"error": err.Error(),
|
||||
"error": err,
|
||||
}).Debug("ValidateSession: user not found")
|
||||
//nolint:nilerr
|
||||
return &proto.ValidateSessionResponse{
|
||||
Valid: false,
|
||||
DeniedReason: "user_not_found",
|
||||
DeniedReason: deniedReasonUserNotFound,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -1759,6 +1809,23 @@ func (s *ProxyServiceServer) ValidateSession(ctx context.Context, req *proto.Val
|
||||
}, nil
|
||||
}
|
||||
|
||||
if reason := userStatusDeniedReason(user); reason != "" {
|
||||
log.WithFields(log.Fields{
|
||||
"domain": domain,
|
||||
"user_id": userID,
|
||||
"reason": reason,
|
||||
}).Debug("ValidateSession: user status denies access")
|
||||
groupIDs, groupNames := pairGroupIDsAndNames(userGroups)
|
||||
return &proto.ValidateSessionResponse{
|
||||
Valid: false,
|
||||
UserId: user.Id,
|
||||
UserEmail: user.Email,
|
||||
DeniedReason: reason,
|
||||
PeerGroupIds: groupIDs,
|
||||
PeerGroupNames: groupNames,
|
||||
}, nil
|
||||
}
|
||||
|
||||
if err := s.checkGroupAccess(service, user); err != nil {
|
||||
log.WithFields(log.Fields{
|
||||
"domain": domain,
|
||||
@@ -1909,6 +1976,18 @@ func (s *ProxyServiceServer) ValidateTunnelPeer(ctx context.Context, req *proto.
|
||||
groupIDs, groupNames := pairGroupIDsAndNames(peerGroups)
|
||||
principalID, displayIdentity := s.getTunnelPeerInfo(ctx, domain, service, peer)
|
||||
|
||||
if reason := s.peerOwnerDeniedReason(ctx, peer); reason != "" {
|
||||
log.WithFields(log.Fields{"domain": domain, "peer_id": peer.ID, "user_id": peer.UserID, "reason": reason}).Debug("ValidateTunnelPeer: owner status denies access")
|
||||
return &proto.ValidateTunnelPeerResponse{
|
||||
Valid: false,
|
||||
UserId: principalID,
|
||||
UserEmail: displayIdentity,
|
||||
DeniedReason: reason,
|
||||
PeerGroupIds: groupIDs,
|
||||
PeerGroupNames: groupNames,
|
||||
}, nil
|
||||
}
|
||||
|
||||
if err := checkPeerGroupAccess(service, groupIDs); err != nil {
|
||||
log.WithFields(log.Fields{"domain": domain, "peer_id": peer.ID, "error": err.Error()}).Debug("ValidateTunnelPeer: access denied")
|
||||
//nolint:nilerr
|
||||
@@ -1944,6 +2023,25 @@ func (s *ProxyServiceServer) ValidateTunnelPeer(ctx context.Context, req *proto.
|
||||
}, nil
|
||||
}
|
||||
|
||||
// peerOwnerDeniedReason gates the mesh fast-path on the account status of the
|
||||
// peer's owning user, so a user blocked after registering a peer loses
|
||||
// mesh-origin access too. Unlinked peers (machine agents) have no owner to gate
|
||||
// on and stay first-class callers. An owner the store cannot resolve denies:
|
||||
// an unavailable lookup must not grant access.
|
||||
func (s *ProxyServiceServer) peerOwnerDeniedReason(ctx context.Context, peer *peer.Peer) string {
|
||||
if peer.UserID == "" {
|
||||
return ""
|
||||
}
|
||||
|
||||
user, err := s.usersManager.GetUser(ctx, peer.UserID)
|
||||
if err != nil {
|
||||
log.WithContext(ctx).Debugf("ValidateTunnelPeer: look up owner %s of peer %s: %v", peer.UserID, peer.ID, err)
|
||||
return deniedReasonUserNotFound
|
||||
}
|
||||
|
||||
return userStatusDeniedReason(user)
|
||||
}
|
||||
|
||||
// getTunnelPeerInfo returns the principal ID and display name for a peer, e.g. a
|
||||
// user or peer ID, and peer name or user email.
|
||||
func (s *ProxyServiceServer) getTunnelPeerInfo(ctx context.Context, domain string, service *rpservice.Service, peer *peer.Peer) (string, string) {
|
||||
|
||||
@@ -350,6 +350,64 @@ func TestValidateUserGroupAccess(t *testing.T) {
|
||||
},
|
||||
expectErr: false,
|
||||
},
|
||||
{
|
||||
name: "user pending approval denied despite group membership",
|
||||
domain: "app.example.com",
|
||||
userID: "user1",
|
||||
proxiesByAccount: map[string][]*service.Service{
|
||||
"account1": {{
|
||||
Domain: "app.example.com",
|
||||
AccountID: "account1",
|
||||
Auth: service.AuthConfig{
|
||||
BearerAuth: &service.BearerAuthConfig{
|
||||
Enabled: true,
|
||||
DistributionGroups: []string{"group1"},
|
||||
},
|
||||
},
|
||||
}},
|
||||
},
|
||||
users: map[string]*types.User{
|
||||
// The approval flow stores a pending user as blocked as well.
|
||||
"user1": {Id: "user1", AccountID: "account1", AutoGroups: []string{"group1"}, Blocked: true, PendingApproval: true},
|
||||
},
|
||||
expectErr: true,
|
||||
expectErrMsg: "user pending approval",
|
||||
},
|
||||
{
|
||||
name: "blocked user denied despite group membership",
|
||||
domain: "app.example.com",
|
||||
userID: "user1",
|
||||
proxiesByAccount: map[string][]*service.Service{
|
||||
"account1": {{
|
||||
Domain: "app.example.com",
|
||||
AccountID: "account1",
|
||||
Auth: service.AuthConfig{
|
||||
BearerAuth: &service.BearerAuthConfig{
|
||||
Enabled: true,
|
||||
DistributionGroups: []string{"group1"},
|
||||
},
|
||||
},
|
||||
}},
|
||||
},
|
||||
users: map[string]*types.User{
|
||||
"user1": {Id: "user1", AccountID: "account1", AutoGroups: []string{"group1"}, Blocked: true},
|
||||
},
|
||||
expectErr: true,
|
||||
expectErrMsg: "user blocked",
|
||||
},
|
||||
{
|
||||
name: "blocked user denied on a service with no auth configured",
|
||||
domain: "app.example.com",
|
||||
userID: "user1",
|
||||
proxiesByAccount: map[string][]*service.Service{
|
||||
"account1": {{Domain: "app.example.com", AccountID: "account1", Auth: service.AuthConfig{}}},
|
||||
},
|
||||
users: map[string]*types.User{
|
||||
"user1": {Id: "user1", AccountID: "account1", Blocked: true},
|
||||
},
|
||||
expectErr: true,
|
||||
expectErrMsg: "user blocked",
|
||||
},
|
||||
{
|
||||
name: "proxy manager error",
|
||||
domain: "app.example.com",
|
||||
@@ -421,17 +479,18 @@ func TestValidateTunnelPeerUserEmailEnrichment(t *testing.T) {
|
||||
storedUserNoEmail := map[string]*types.User{userID: {Id: userID, AccountID: accountID, Email: ""}}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
peerUserID string
|
||||
storedUsers map[string]*types.User
|
||||
storedErr error
|
||||
noIdP bool
|
||||
idpEmail string
|
||||
idpHasData bool
|
||||
idpErr error
|
||||
expectEmail string
|
||||
expectUserID string
|
||||
expectIdPHit bool
|
||||
name string
|
||||
peerUserID string
|
||||
storedUsers map[string]*types.User
|
||||
storedErr error
|
||||
noIdP bool
|
||||
idpEmail string
|
||||
idpHasData bool
|
||||
idpErr error
|
||||
expectEmail string
|
||||
expectUserID string
|
||||
expectIdPHit bool
|
||||
expectDeniedReason string
|
||||
}{
|
||||
{
|
||||
name: "idp email wins over stored email",
|
||||
@@ -490,14 +549,17 @@ func TestValidateTunnelPeerUserEmailEnrichment(t *testing.T) {
|
||||
expectIdPHit: true,
|
||||
},
|
||||
{
|
||||
name: "idp email when stored user missing keeps peer.UserID as principal",
|
||||
peerUserID: userID,
|
||||
storedUsers: map[string]*types.User{},
|
||||
idpEmail: "idp@example.com",
|
||||
idpHasData: true,
|
||||
expectEmail: "idp@example.com",
|
||||
expectUserID: userID,
|
||||
expectIdPHit: true,
|
||||
// The identity still resolves from the IdP, but an owner the store
|
||||
// cannot resolve denies the fast-path rather than granting it.
|
||||
name: "idp email when stored user missing keeps peer.UserID as principal",
|
||||
peerUserID: userID,
|
||||
storedUsers: map[string]*types.User{},
|
||||
idpEmail: "idp@example.com",
|
||||
idpHasData: true,
|
||||
expectEmail: "idp@example.com",
|
||||
expectUserID: userID,
|
||||
expectIdPHit: true,
|
||||
expectDeniedReason: "user_not_found",
|
||||
},
|
||||
{
|
||||
name: "unlinked peer uses peer name and never consults idp",
|
||||
@@ -545,7 +607,8 @@ func TestValidateTunnelPeerUserEmailEnrichment(t *testing.T) {
|
||||
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
assert.True(t, resp.GetValid(), "expected access granted")
|
||||
assert.Equal(t, tt.expectDeniedReason == "", resp.GetValid(), "unexpected access decision")
|
||||
assert.Equal(t, tt.expectDeniedReason, resp.GetDeniedReason())
|
||||
assert.Equal(t, tt.expectEmail, resp.GetUserEmail())
|
||||
assert.Equal(t, tt.expectUserID, resp.GetUserId())
|
||||
|
||||
@@ -562,6 +625,78 @@ func TestValidateTunnelPeerUserEmailEnrichment(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateTunnelPeerOwnerStatus verifies that the mesh fast-path gates on
|
||||
// the account status of the peer's owning user. A peer whose owner was blocked
|
||||
// after the peer registered must lose access, while an unlinked machine peer
|
||||
// keeps it.
|
||||
func TestValidateTunnelPeerOwnerStatus(t *testing.T) {
|
||||
const (
|
||||
domain = "app.example.com"
|
||||
accountID = "account1"
|
||||
peerID = "peer1"
|
||||
peerName = "peer-display-name"
|
||||
userID = "user1"
|
||||
)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
peerUserID string
|
||||
owner *types.User
|
||||
expectDeniedReason string
|
||||
}{
|
||||
{
|
||||
name: "active owner allowed",
|
||||
peerUserID: userID,
|
||||
owner: &types.User{Id: userID, AccountID: accountID, Email: "user@example.com"},
|
||||
},
|
||||
{
|
||||
name: "owner pending approval denied",
|
||||
peerUserID: userID,
|
||||
owner: &types.User{Id: userID, AccountID: accountID, Email: "user@example.com", Blocked: true, PendingApproval: true},
|
||||
expectDeniedReason: "pending_approval",
|
||||
},
|
||||
{
|
||||
name: "owner blocked after registering the peer denied",
|
||||
peerUserID: userID,
|
||||
owner: &types.User{Id: userID, AccountID: accountID, Email: "user@example.com", Blocked: true},
|
||||
expectDeniedReason: "user_blocked",
|
||||
},
|
||||
{
|
||||
name: "unlinked machine peer stays allowed",
|
||||
peerUserID: "",
|
||||
owner: &types.User{Id: userID, AccountID: accountID, Blocked: true},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
svc := &service.Service{Domain: domain, AccountID: accountID}
|
||||
server := &ProxyServiceServer{
|
||||
serviceManager: &mockReverseProxyManager{
|
||||
proxiesByAccount: map[string][]*service.Service{accountID: {svc}},
|
||||
},
|
||||
peersManager: &mockTunnelPeersManager{
|
||||
peer: &peer.Peer{ID: peerID, Name: peerName, UserID: tt.peerUserID},
|
||||
},
|
||||
usersManager: &mockUsersManager{users: map[string]*types.User{userID: tt.owner}},
|
||||
}
|
||||
|
||||
resp, err := server.ValidateTunnelPeer(context.Background(), &proto.ValidateTunnelPeerRequest{
|
||||
Domain: domain,
|
||||
TunnelIp: "100.64.0.1",
|
||||
})
|
||||
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
assert.Equal(t, tt.expectDeniedReason, resp.GetDeniedReason())
|
||||
assert.Equal(t, tt.expectDeniedReason == "", resp.GetValid(), "unexpected access decision")
|
||||
if tt.expectDeniedReason != "" {
|
||||
assert.Empty(t, resp.GetSessionToken(), "a denied peer must not receive a session token")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAccountProxyByDomain(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
@@ -46,6 +46,7 @@ func setupValidateSessionTest(t *testing.T) *validateSessionTestSetup {
|
||||
proxyService.SetServiceManager(serviceManager)
|
||||
|
||||
createTestProxies(t, ctx, testStore)
|
||||
createStatusTestUsers(t, ctx, testStore)
|
||||
|
||||
return &validateSessionTestSetup{
|
||||
proxyService: proxyService,
|
||||
@@ -91,6 +92,82 @@ func createTestProxies(t *testing.T, ctx context.Context, testStore store.Store)
|
||||
},
|
||||
}
|
||||
require.NoError(t, testStore.CreateService(ctx, restrictedProxy))
|
||||
|
||||
// Distributed to the account's "All" group, the configuration that hands a
|
||||
// service to every user in the account.
|
||||
allUsersProxy := &service.Service{
|
||||
ID: "allUsersProxyId",
|
||||
AccountID: "testAccountId",
|
||||
Name: "All Users Proxy",
|
||||
Domain: "all-users-proxy.example.com",
|
||||
Enabled: true,
|
||||
SessionPrivateKey: privKey,
|
||||
SessionPublicKey: pubKey,
|
||||
Auth: service.AuthConfig{
|
||||
BearerAuth: &service.BearerAuthConfig{
|
||||
Enabled: true,
|
||||
DistributionGroups: []string{allUsersGroupID},
|
||||
},
|
||||
},
|
||||
}
|
||||
require.NoError(t, testStore.CreateService(ctx, allUsersProxy))
|
||||
}
|
||||
|
||||
const (
|
||||
allUsersGroupID = "allUsersGroupId"
|
||||
pendingUserID = "pendingUserId"
|
||||
blockedUserID = "blockedUserId"
|
||||
pendingAllUsersID = "pendingAllUsersUserId"
|
||||
)
|
||||
|
||||
// createStatusTestUsers adds the users whose account status must keep them out
|
||||
// of a proxy session. A user awaiting approval is persisted as both blocked and
|
||||
// pending approval, the way the approval flow stores one.
|
||||
func createStatusTestUsers(t *testing.T, ctx context.Context, testStore store.Store) {
|
||||
t.Helper()
|
||||
|
||||
require.NoError(t, testStore.CreateGroup(ctx, &types.Group{
|
||||
ID: allUsersGroupID,
|
||||
AccountID: "testAccountId",
|
||||
Name: "All",
|
||||
Issued: types.GroupIssuedAPI,
|
||||
}))
|
||||
|
||||
users := []*types.User{
|
||||
{
|
||||
Id: pendingUserID,
|
||||
AccountID: "testAccountId",
|
||||
Role: types.UserRoleUser,
|
||||
AutoGroups: []string{"allowedGroupId"},
|
||||
Blocked: true,
|
||||
PendingApproval: true,
|
||||
Issued: "api",
|
||||
CreatedAt: time.Now(),
|
||||
},
|
||||
{
|
||||
Id: pendingAllUsersID,
|
||||
AccountID: "testAccountId",
|
||||
Role: types.UserRoleUser,
|
||||
AutoGroups: []string{allUsersGroupID},
|
||||
Blocked: true,
|
||||
PendingApproval: true,
|
||||
Issued: "api",
|
||||
CreatedAt: time.Now(),
|
||||
},
|
||||
{
|
||||
Id: blockedUserID,
|
||||
AccountID: "testAccountId",
|
||||
Role: types.UserRoleUser,
|
||||
AutoGroups: []string{"allowedGroupId"},
|
||||
Blocked: true,
|
||||
PendingApproval: false,
|
||||
Issued: "api",
|
||||
CreatedAt: time.Now(),
|
||||
},
|
||||
}
|
||||
for _, user := range users {
|
||||
require.NoError(t, testStore.SaveUser(ctx, user))
|
||||
}
|
||||
}
|
||||
|
||||
func generateSessionKeyPair(t *testing.T) (string, string) {
|
||||
@@ -149,6 +226,114 @@ func TestValidateSession_UserNotInAllowedGroup(t *testing.T) {
|
||||
assert.Empty(t, resp.GetPeerGroupIds(), "PeerGroupIds must mirror the resolved user's actual (empty) memberships on denial")
|
||||
}
|
||||
|
||||
// TestValidateSession_PendingApprovalUserDenied covers a user who is a member of
|
||||
// the service's distribution group but is still waiting for an administrator to
|
||||
// approve the account. Group membership alone must not open the service.
|
||||
func TestValidateSession_PendingApprovalUserDenied(t *testing.T) {
|
||||
setup := setupValidateSessionTest(t)
|
||||
defer setup.cleanup()
|
||||
|
||||
proxy, err := setup.store.GetServiceByID(context.Background(), store.LockingStrengthNone, "testAccountId", "restrictedProxyId")
|
||||
require.NoError(t, err)
|
||||
|
||||
token := createSessionToken(t, proxy.SessionPrivateKey, pendingUserID, "restricted-proxy.example.com")
|
||||
|
||||
resp, err := setup.proxyService.ValidateSession(context.Background(), &proto.ValidateSessionRequest{
|
||||
Domain: "restricted-proxy.example.com",
|
||||
SessionToken: token,
|
||||
})
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.Valid, "User pending approval should be denied")
|
||||
assert.Equal(t, "pending_approval", resp.DeniedReason)
|
||||
assert.Equal(t, pendingUserID, resp.UserId)
|
||||
assert.Equal(t, []string{"allowedGroupId"}, resp.GetPeerGroupIds(), "PeerGroupIds must mirror the resolved user's group memberships on denial")
|
||||
assert.Equal(t, []string{"Allowed Group"}, resp.GetPeerGroupNames(), "PeerGroupNames must pair with PeerGroupIds on denial")
|
||||
}
|
||||
|
||||
// TestValidateSession_PendingApprovalUserInAllUsersGroupDenied covers the same
|
||||
// user against a service distributed to the account's "All" group, where every
|
||||
// user of the account is a member by default.
|
||||
func TestValidateSession_PendingApprovalUserInAllUsersGroupDenied(t *testing.T) {
|
||||
setup := setupValidateSessionTest(t)
|
||||
defer setup.cleanup()
|
||||
|
||||
proxy, err := setup.store.GetServiceByID(context.Background(), store.LockingStrengthNone, "testAccountId", "allUsersProxyId")
|
||||
require.NoError(t, err)
|
||||
|
||||
token := createSessionToken(t, proxy.SessionPrivateKey, pendingAllUsersID, "all-users-proxy.example.com")
|
||||
|
||||
resp, err := setup.proxyService.ValidateSession(context.Background(), &proto.ValidateSessionRequest{
|
||||
Domain: "all-users-proxy.example.com",
|
||||
SessionToken: token,
|
||||
})
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.Valid, "User pending approval should be denied even in the All Users group")
|
||||
assert.Equal(t, "pending_approval", resp.DeniedReason)
|
||||
assert.Equal(t, pendingAllUsersID, resp.UserId)
|
||||
assert.Equal(t, []string{allUsersGroupID}, resp.GetPeerGroupIds(), "PeerGroupIds must mirror the resolved user's group memberships on denial")
|
||||
}
|
||||
|
||||
// TestValidateSession_BlockedUserDenied covers a user blocked after having been
|
||||
// approved, so PendingApproval is false and only the blocked flag is set.
|
||||
func TestValidateSession_BlockedUserDenied(t *testing.T) {
|
||||
setup := setupValidateSessionTest(t)
|
||||
defer setup.cleanup()
|
||||
|
||||
proxy, err := setup.store.GetServiceByID(context.Background(), store.LockingStrengthNone, "testAccountId", "restrictedProxyId")
|
||||
require.NoError(t, err)
|
||||
|
||||
token := createSessionToken(t, proxy.SessionPrivateKey, blockedUserID, "restricted-proxy.example.com")
|
||||
|
||||
resp, err := setup.proxyService.ValidateSession(context.Background(), &proto.ValidateSessionRequest{
|
||||
Domain: "restricted-proxy.example.com",
|
||||
SessionToken: token,
|
||||
})
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.Valid, "Blocked user should be denied")
|
||||
assert.Equal(t, "user_blocked", resp.DeniedReason)
|
||||
assert.Equal(t, blockedUserID, resp.UserId)
|
||||
}
|
||||
|
||||
// TestValidateSession_UserAllowedAfterApproval walks the same session token
|
||||
// through the approval transition: denied while pending, allowed once an
|
||||
// administrator clears both flags.
|
||||
func TestValidateSession_UserAllowedAfterApproval(t *testing.T) {
|
||||
setup := setupValidateSessionTest(t)
|
||||
defer setup.cleanup()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
proxy, err := setup.store.GetServiceByID(ctx, store.LockingStrengthNone, "testAccountId", "restrictedProxyId")
|
||||
require.NoError(t, err)
|
||||
|
||||
token := createSessionToken(t, proxy.SessionPrivateKey, pendingUserID, "restricted-proxy.example.com")
|
||||
req := &proto.ValidateSessionRequest{
|
||||
Domain: "restricted-proxy.example.com",
|
||||
SessionToken: token,
|
||||
}
|
||||
|
||||
resp, err := setup.proxyService.ValidateSession(ctx, req)
|
||||
require.NoError(t, err)
|
||||
require.False(t, resp.Valid, "User pending approval should be denied before approval")
|
||||
assert.Equal(t, "pending_approval", resp.DeniedReason)
|
||||
|
||||
user, err := setup.store.GetUserByUserID(ctx, store.LockingStrengthNone, pendingUserID)
|
||||
require.NoError(t, err)
|
||||
user.PendingApproval = false
|
||||
user.Blocked = false
|
||||
require.NoError(t, setup.store.SaveUser(ctx, user))
|
||||
|
||||
resp, err = setup.proxyService.ValidateSession(ctx, req)
|
||||
require.NoError(t, err)
|
||||
assert.True(t, resp.Valid, "Approved user should be allowed access")
|
||||
assert.Empty(t, resp.DeniedReason)
|
||||
assert.Equal(t, pendingUserID, resp.UserId)
|
||||
assert.Equal(t, []string{"allowedGroupId"}, resp.GetPeerGroupIds())
|
||||
}
|
||||
|
||||
func TestValidateSession_UserInDifferentAccount(t *testing.T) {
|
||||
setup := setupValidateSessionTest(t)
|
||||
defer setup.cleanup()
|
||||
|
||||
@@ -2,6 +2,7 @@ package proxy
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
@@ -108,7 +109,7 @@ func (h *AuthCallbackHandler) handleCallback(w http.ResponseWriter, r *http.Requ
|
||||
redirectURL.Scheme = "https"
|
||||
query := redirectURL.Query()
|
||||
query.Set("error", "access_denied")
|
||||
query.Set("error_description", "Service configuration error")
|
||||
query.Set("error_description", sessionTokenErrorDescription(err))
|
||||
redirectURL.RawQuery = query.Encode()
|
||||
http.Redirect(w, r, redirectURL.String(), http.StatusFound)
|
||||
return
|
||||
@@ -124,6 +125,21 @@ func (h *AuthCallbackHandler) handleCallback(w http.ResponseWriter, r *http.Requ
|
||||
http.Redirect(w, r, redirectURL.String(), http.StatusFound)
|
||||
}
|
||||
|
||||
// sessionTokenErrorDescription maps a session token failure to the text the
|
||||
// proxy renders on its access denied page. Account status denials get a message
|
||||
// the user can act on, while everything else stays generic so a lookup or
|
||||
// signing failure does not describe management internals to the browser.
|
||||
func sessionTokenErrorDescription(err error) string {
|
||||
switch {
|
||||
case errors.Is(err, nbgrpc.ErrUserPendingApproval):
|
||||
return "Your account is pending approval by an administrator"
|
||||
case errors.Is(err, nbgrpc.ErrUserBlocked):
|
||||
return "Your account is blocked"
|
||||
default:
|
||||
return "Service configuration error"
|
||||
}
|
||||
}
|
||||
|
||||
func extractUserIDFromToken(ctx context.Context, provider *oidc.Provider, config nbgrpc.ProxyOIDCConfig, token *oauth2.Token) string {
|
||||
rawIDToken, ok := token.Extra("id_token").(string)
|
||||
if !ok {
|
||||
|
||||
@@ -360,6 +360,31 @@ func createTestAccountsAndUsers(t *testing.T, ctx context.Context, testStore sto
|
||||
Issued: "api",
|
||||
}
|
||||
require.NoError(t, testStore.SaveUser(ctx, allowedUser))
|
||||
|
||||
// A user awaiting approval is stored as blocked and pending approval, and
|
||||
// carries the same group membership as the approved one.
|
||||
pendingUser := &types.User{
|
||||
Id: "pendingUserId",
|
||||
AccountID: "testAccountId",
|
||||
Role: types.UserRoleUser,
|
||||
AutoGroups: []string{"allowedGroupId"},
|
||||
Blocked: true,
|
||||
PendingApproval: true,
|
||||
CreatedAt: time.Now(),
|
||||
Issued: "api",
|
||||
}
|
||||
require.NoError(t, testStore.SaveUser(ctx, pendingUser))
|
||||
|
||||
blockedUser := &types.User{
|
||||
Id: "blockedUserId",
|
||||
AccountID: "testAccountId",
|
||||
Role: types.UserRoleUser,
|
||||
AutoGroups: []string{"allowedGroupId"},
|
||||
Blocked: true,
|
||||
CreatedAt: time.Now(),
|
||||
Issued: "api",
|
||||
}
|
||||
require.NoError(t, testStore.SaveUser(ctx, blockedUser))
|
||||
}
|
||||
|
||||
// testServiceManager is a minimal implementation for testing.
|
||||
@@ -490,6 +515,58 @@ func TestAuthCallback_UserAllowedToLogin(t *testing.T) {
|
||||
require.Empty(t, parsedLocation.Query().Get("error"), "Should not have error parameter")
|
||||
}
|
||||
|
||||
// TestAuthCallback_UserDeniedByAccountStatus asserts that a user whose account
|
||||
// is pending approval or blocked never receives a session token from the OIDC
|
||||
// callback, and that the redirect carries a description the proxy can render.
|
||||
func TestAuthCallback_UserDeniedByAccountStatus(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
subject string
|
||||
expectErrorDesc string
|
||||
}{
|
||||
{
|
||||
name: "pending approval",
|
||||
subject: "pendingUserId",
|
||||
expectErrorDesc: "Your account is pending approval by an administrator",
|
||||
},
|
||||
{
|
||||
name: "blocked",
|
||||
subject: "blockedUserId",
|
||||
expectErrorDesc: "Your account is blocked",
|
||||
},
|
||||
{
|
||||
name: "unknown to management",
|
||||
subject: "userMissingFromStoreId",
|
||||
expectErrorDesc: "Service configuration error",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
setup := setupAuthCallbackTest(t)
|
||||
defer setup.cleanup()
|
||||
|
||||
setup.oidcServer.tokenSubject = tt.subject
|
||||
|
||||
state := createTestState(t, setup.proxyService, "https://test-proxy.example.com/dashboard")
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/reverse-proxy/callback?code=test-auth-code&state="+url.QueryEscape(state), nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
setup.router.ServeHTTP(rec, req)
|
||||
|
||||
require.Equal(t, http.StatusFound, rec.Code)
|
||||
|
||||
parsedLocation, err := url.Parse(rec.Header().Get("Location"))
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Empty(t, parsedLocation.Query().Get("session_token"), "Denied user must not receive a session token")
|
||||
require.Equal(t, "access_denied", parsedLocation.Query().Get("error"))
|
||||
require.Equal(t, tt.expectErrorDesc, parsedLocation.Query().Get("error_description"))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthCallback_ProxyNotFound(t *testing.T) {
|
||||
setup := setupAuthCallbackTest(t)
|
||||
defer setup.cleanup()
|
||||
|
||||
Reference in New Issue
Block a user