diff --git a/management/server/types/user.go b/management/server/types/user.go index 02358ebc2..bebc20ea4 100644 --- a/management/server/types/user.go +++ b/management/server/types/user.go @@ -285,6 +285,25 @@ func (u *User) EncryptSensitiveData(enc *crypt.FieldEncrypt) error { return nil } +func MaskEmail(email string) string { + local, domain, found := strings.Cut(email, "@") + if !found || local == "" || domain == "" { + return "" + } + + // Runes, not bytes, so a non-ASCII local part is not cut mid-character. + runes := []rune(local) + + // Keeping the first two and the last needs a local part of at least four to + // hide anything at all: at three or fewer those are the whole of it, and the + // address would be recoverable in full from what is meant to conceal it. + if len(runes) < 4 { + return "****@" + domain + } + + return string(runes[:2]) + "****" + string(runes[len(runes)-1]) + "@" + domain +} + // DecryptSensitiveData decrypts the user's sensitive fields (Email and Name) in place. func (u *User) DecryptSensitiveData(enc *crypt.FieldEncrypt) error { if enc == nil { diff --git a/management/server/types/user_test.go b/management/server/types/user_test.go index e11df96aa..1b3ce7ce6 100644 --- a/management/server/types/user_test.go +++ b/management/server/types/user_test.go @@ -296,3 +296,144 @@ func TestUser_EncryptDecryptRoundTrip(t *testing.T) { }) } } + +func TestMaskEmail(t *testing.T) { + testCases := []struct { + name string + email string + expected string + }{ + { + name: "ordinary address keeps the first two, the last, and the domain", + email: "admin@example.com", + expected: "ad****n@example.com", + }, + { + name: "four characters is the shortest local part that reveals anything", + email: "abcd@example.com", + expected: "ab****d@example.com", + }, + { + name: "three character local part is masked whole, since a lead and tail would be all of it", + email: "abc@example.com", + expected: "****@example.com", + }, + { + name: "two character local part is masked whole", + email: "ab@example.com", + expected: "****@example.com", + }, + { + name: "single character local part is masked whole", + email: "a@b.co", + expected: "****@b.co", + }, + { + name: "mask width does not report the length it stands in for", + email: "a.very.long.local.part@example.com", + expected: "a.****t@example.com", + }, + { + name: "a local part far longer than the mask is still reduced to three characters", + email: "finance.department.notifications.owner.account@example.com", + expected: "fi****t@example.com", + }, + { + name: "plus addressing is masked along with the rest of the local part", + email: "admin+netbird@example.com", + expected: "ad****d@example.com", + }, + { + name: "separators inside the local part are not treated specially", + email: "first.last-name_x@example.com", + expected: "fi****x@example.com", + }, + { + name: "case is preserved rather than normalised", + email: "Admin@Example.COM", + expected: "Ad****n@Example.COM", + }, + { + name: "subdomains stay intact", + email: "owner@mail.corp.example.com", + expected: "ow****r@mail.corp.example.com", + }, + { + name: "german umlauts count as single characters", + email: "müller@example.de", + expected: "mü****r@example.de", + }, + { + name: "cyrillic local part is cut on runes", + email: "иванов@example.ru", + expected: "ив****в@example.ru", + }, + { + name: "cjk local part of three runes is masked whole, counted in runes not bytes", + email: "用户名@example.cn", + expected: "****@example.cn", + }, + { + name: "cjk local part of four runes reveals the first two and the last", + email: "用户名字@example.cn", + expected: "用户****字@example.cn", + }, + { + name: "arabic local part is cut on runes", + email: "مستخدم@example.sa", + expected: "مس****م@example.sa", + }, + { + name: "two rune non-ascii local part is masked whole", + email: "ää@example.de", + expected: "****@example.de", + }, + { + name: "astral plane runes are not split into surrogates", + email: "a🎉bc@example.com", + expected: "a🎉****c@example.com", + }, + { + name: "a non-ascii domain is left alone", + email: "admin@münchen.example", + expected: "ad****n@münchen.example", + }, + { + name: "only the first separator splits, so a second stays in the domain", + email: "a@b@example.com", + expected: "****@b@example.com", + }, + { + name: "empty email has nothing to mask", + email: "", + expected: "", + }, + { + name: "value without a separator is not an address", + email: "not-an-email", + expected: "", + }, + { + name: "missing local part is not an address", + email: "@example.com", + expected: "", + }, + { + name: "missing domain is not an address", + email: "admin@", + expected: "", + }, + { + name: "a bare separator is not an address", + email: "@", + expected: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, MaskEmail(tc.email)) + }) + } + +} diff --git a/management/server/user.go b/management/server/user.go index 823c1b2e4..3510a624b 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -1448,6 +1448,25 @@ func (am *DefaultAccountManager) deleteRegularUser(ctx context.Context, accountI return updateAccountPeers, nil } +// pendingApprovalError refuses a user awaiting approval, naming the owner who +// can approve them when their address resolves. Failing to resolve one is not a +// reason to withhold the refusal, so the lookup is best effort. +func (am *DefaultAccountManager) pendingApprovalError(ctx context.Context, accountID string) error { + owner, err := am.GetOwnerInfo(ctx, accountID) + if err != nil { + log.WithContext(ctx).Debugf("pending approval refusal: owner of account %s did not resolve: %v", accountID, err) + return status.NewUserPendingApprovalError() + } + + masked := types.MaskEmail(owner.Email) + if masked == "" { + log.WithContext(ctx).Debugf("pending approval refusal: no address found for the owner of account %s", accountID) + return status.NewUserPendingApprovalError() + } + + return status.NewUserPendingApprovalByOwnerError(masked) +} + // GetOwnerInfo retrieves the owner information for a given account ID. func (am *DefaultAccountManager) GetOwnerInfo(ctx context.Context, accountID string) (*types.UserInfo, error) { owner, err := am.Store.GetAccountOwner(ctx, store.LockingStrengthNone, accountID) @@ -1505,6 +1524,14 @@ func (am *DefaultAccountManager) GetCurrentUserInfo(ctx context.Context, userAut return nil, err } + // A user pending approval is blocked too, and the dashboard needs to tell + // the two apart: one is a dead end, the other resolves by itself once the + // owner acts. Naming that owner needs the address the IdP holds, which is + // why this is answered here rather than in the permission gate. + if user.IsBlocked() && user.PendingApproval { + return nil, am.pendingApprovalError(ctx, user.AccountID) + } + if user.IsBlocked() { return nil, status.NewUserBlockedError() } diff --git a/management/server/user_test.go b/management/server/user_test.go index ec0bbc54e..2d1a5f1e9 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -1779,6 +1779,42 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { } require.NoError(t, store.SaveAccount(context.Background(), account2)) + account3 := newAccountWithId(context.Background(), "account3", "account3Owner", "", "owner@example.com", "", false) + account3.Users["pending-user"] = &types.User{ + Id: "pending-user", + AccountID: account3.Id, + Role: types.UserRoleUser, + Blocked: true, + PendingApproval: true, + } + require.NoError(t, store.SaveAccount(context.Background(), account3)) + + // The owner has no address to name, so the refusal falls back to the generic one. + account4 := newAccountWithId(context.Background(), "account4", "account4Owner", "", "", "", false) + account4.Users["pending-user-without-owner-email"] = &types.User{ + Id: "pending-user-without-owner-email", + AccountID: account4.Id, + Role: types.UserRoleUser, + Blocked: true, + PendingApproval: true, + } + require.NoError(t, store.SaveAccount(context.Background(), account4)) + + // No user holds the owner role, so the owner lookup itself fails. + account5 := newAccountWithId(context.Background(), "account5", "account5Admin", "", "", "", false) + account5.Users["account5Admin"].Role = types.UserRoleAdmin + account5.Users["pending-user-without-owner"] = &types.User{ + Id: "pending-user-without-owner", + AccountID: account5.Id, + Role: types.UserRoleUser, + Blocked: true, + PendingApproval: true, + } + require.NoError(t, store.SaveAccount(context.Background(), account5)) + + account6 := newAccountWithId(context.Background(), "account6", "account6Owner", "", "stranger@example.com", "", false) + require.NoError(t, store.SaveAccount(context.Background(), account6)) + permissionsManager := permissions.NewManager(store) am := DefaultAccountManager{ Store: store, @@ -1812,6 +1848,34 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) { userAuth: auth.UserAuth{AccountId: account1.Id, UserId: "service-user"}, expectedErr: status.NewPermissionDeniedError(), }, + { + name: "pending approval names the owner", + userAuth: auth.UserAuth{AccountId: account3.Id, UserId: "pending-user"}, + expectedErr: status.NewUserPendingApprovalByOwnerError("ow****r@example.com"), + }, + { + name: "pending approval without an owner address", + userAuth: auth.UserAuth{AccountId: account4.Id, UserId: "pending-user-without-owner-email"}, + expectedErr: status.NewUserPendingApprovalError(), + }, + { + name: "pending approval without an owner", + userAuth: auth.UserAuth{AccountId: account5.Id, UserId: "pending-user-without-owner"}, + expectedErr: status.NewUserPendingApprovalError(), + }, + { + // The account claim points at an account the caller is not in. The + // owner named has to be the one of the account holding the caller's + // own record, never the one the claim asks for. + name: "pending approval ignores a mismatched account claim", + userAuth: auth.UserAuth{AccountId: account6.Id, UserId: "pending-user"}, + expectedErr: status.NewUserPendingApprovalByOwnerError("ow****r@example.com"), + }, + { + name: "blocked user answers before the account claim is validated", + userAuth: auth.UserAuth{AccountId: account6.Id, UserId: "blocked-user"}, + expectedErr: status.NewUserBlockedError(), + }, { name: "owner user", userAuth: auth.UserAuth{AccountId: account1.Id, UserId: "account1Owner"}, diff --git a/shared/management/status/error.go b/shared/management/status/error.go index e31663450..4249f27de 100644 --- a/shared/management/status/error.go +++ b/shared/management/status/error.go @@ -135,6 +135,11 @@ func NewUserPendingApprovalError() error { return Errorf(PermissionDenied, "user is pending approval") } +// NewUserPendingApprovalByOwnerError creates a new Error with PermissionDenied type for a blocked user pending approval, naming the masked address of the owner who can approve them +func NewUserPendingApprovalByOwnerError(ownerEmail string) error { + return Errorf(PermissionDenied, "user is pending approval by owner %s", ownerEmail) +} + // NewPeerNotRegisteredError creates a new Error with Unauthenticated type unregistered peer func NewPeerNotRegisteredError() error { return Errorf(Unauthenticated, "peer is not registered")