Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Commit

Permalink
Rename deprovision to ban endpoint (#802)
Browse files Browse the repository at this point in the history
* Rename `deprovision` to `ban` endpoint

Create a new `ban` endpoint and internally
forward all incoming requests from `deprovision`
to `ban`
Also, rename all references of `deprovision` to `ban`,
including a new column `ban` in the database, containing
the same values as its sibling column `deprovision`.
Both columns should be updated in parallel until the transition
to the new endpoint is complete.

Fixes #ODC117

Signed-off-by: Xavier Coulon <[email protected]>

* more renaming

Signed-off-by: Xavier Coulon <[email protected]>

* gofmt

Signed-off-by: Xavier Coulon <[email protected]>
  • Loading branch information
xcoulon authored and alexeykazakov committed Mar 13, 2019
1 parent a11b741 commit 4089f9c
Show file tree
Hide file tree
Showing 39 changed files with 349 additions and 207 deletions.
6 changes: 3 additions & 3 deletions application/service/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,13 @@ type UserProfileService interface {

type UserService interface {
DeactivateUser(ctx context.Context, username string) (*account.Identity, error)
DeprovisionUser(ctx context.Context, username string) (*account.Identity, error)
BanUser(ctx context.Context, username string) (*account.Identity, error)
UserInfo(ctx context.Context, identityID uuid.UUID) (*account.User, *account.Identity, error)
LoadContextIdentityAndUser(ctx context.Context) (*account.Identity, error)
LoadContextIdentityIfNotDeprovisioned(ctx context.Context) (*account.Identity, error)
LoadContextIdentityIfNotBanned(ctx context.Context) (*account.Identity, error)
ContextIdentityIfExists(ctx context.Context) (uuid.UUID, error)
IdentityByUsernameAndEmail(ctx context.Context, username, email string) (*account.Identity, error)
ResetDeprovision(ctx context.Context, user account.User) error
ResetBan(ctx context.Context, user account.User) error
HardDeleteUser(ctx context.Context, identity account.Identity) error
}

Expand Down
6 changes: 3 additions & 3 deletions authentication/account/repository/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"github.com/goadesign/goa"
"github.com/jinzhu/gorm"
errs "github.com/pkg/errors"
"github.com/satori/go.uuid"
uuid "github.com/satori/go.uuid"
)

const (
Expand Down Expand Up @@ -413,7 +413,7 @@ WHERE
identities.user_id = users.id
AND identities.username LIKE ?
AND identities.deleted_at IS NULL
AND users.deprovisioned IS false
AND users.banned IS false
AND users.deleted_at IS NULL
UNION SELECT
identities.id AS identity_id,
Expand All @@ -425,7 +425,7 @@ WHERE
identities.user_id = users.id
AND identities.deleted_at IS NULL
AND users.deleted_at IS NULL
AND users.deprovisioned IS false
AND users.banned IS false
AND (LOWER(users.full_name) LIKE ?
OR (LOWER(users.email) LIKE ? AND users.email_private is false))) users LIMIT ?`

Expand Down
5 changes: 3 additions & 2 deletions authentication/account/repository/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ type User struct {
Company string // The (optional) Company of the User
FeatureLevel string // the level of features that the user opted in (to access unreleased features). Defaults to `released` so no non-released feature is enabled for the user.
Cluster string // The OpenShift cluster allocated to the user.
// Whether the user has been deprovisioned
Deprovisioned bool `gorm:"column:deprovisioned"`
// Whether the user has been banned
Deprovisioned bool `gorm:"column:deprovisioned"` // for backward compatibility
Banned bool `gorm:"column:banned"`
Active bool `gorm:"column:active"`
Identities []Identity // has many Identities from different IDPs
ContextInformation account.ContextInformation `sql:"type:jsonb"` // context information of the user activity
Expand Down
21 changes: 11 additions & 10 deletions authentication/account/service/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ type userServiceImpl struct {
base.BaseService
}

// ResetDeprovisioned sets User.Deprovisioned to false
func (s *userServiceImpl) ResetDeprovision(ctx context.Context, user repository.User) error {
user.Deprovisioned = false
// ResetBanned sets User.Banned to false
func (s *userServiceImpl) ResetBan(ctx context.Context, user repository.User) error {
user.Banned = false
return s.ExecuteInTransaction(func() error {
return s.Repositories().Users().Save(ctx, &user)
})
Expand Down Expand Up @@ -76,7 +76,7 @@ func (s *userServiceImpl) UserInfo(ctx context.Context, identityID uuid.UUID) (*
return &identity.User, identity, nil
}

func (s *userServiceImpl) DeprovisionUser(ctx context.Context, username string) (*repository.Identity, error) {
func (s *userServiceImpl) BanUser(ctx context.Context, username string) (*repository.Identity, error) {

var identity *repository.Identity
err := s.ExecuteInTransaction(func() error {
Expand All @@ -93,7 +93,8 @@ func (s *userServiceImpl) DeprovisionUser(ctx context.Context, username string)
}

identity = &identities[0]
identity.User.Deprovisioned = true
identity.User.Banned = true
identity.User.Deprovisioned = true // for backward compatibility

return s.Repositories().Users().Save(ctx, &identity.User)
})
Expand Down Expand Up @@ -216,15 +217,15 @@ func (s *userServiceImpl) LoadContextIdentityAndUser(ctx context.Context) (*repo
return identity, err
}

// LoadContextIdentityIfNotDeprovisioned returns the same identity as LoadContextIdentityAndUser()
// if the user is not deprovisioned. Returns an Unauthorized error if the user is deprovisioned.
func (s *userServiceImpl) LoadContextIdentityIfNotDeprovisioned(ctx context.Context) (*repository.Identity, error) {
// LoadContextIdentityIfNotBanned returns the same identity as LoadContextIdentityAndUser()
// if the user is not banned. Returns an Unauthorized error if the user is banned.
func (s *userServiceImpl) LoadContextIdentityIfNotBanned(ctx context.Context) (*repository.Identity, error) {
identity, err := s.LoadContextIdentityAndUser(ctx)
if err != nil {
return nil, err
}
if identity.User.Deprovisioned {
return nil, errors.NewUnauthorizedError("user deprovisioned")
if identity.User.Banned {
return nil, errors.NewUnauthorizedError("user banned")
}
return identity, err
}
Expand Down
49 changes: 24 additions & 25 deletions authentication/account/service/user_blackbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,27 @@ func TestUserService(t *testing.T) {
suite.Run(t, &userServiceBlackboxTestSuite{DBTestSuite: gormtestsupport.NewDBTestSuite()})
}

func (s *userServiceBlackboxTestSuite) TestDeprovisionUnknownUserFails() {
}

func (s *userServiceBlackboxTestSuite) TestDeprovision() {
func (s *userServiceBlackboxTestSuite) TestBanUser() {

s.T().Run("ok", func(t *testing.T) {
userToDeprovision := s.Graph.CreateUser()
userToBan := s.Graph.CreateUser()
userToStayIntact := s.Graph.CreateUser()

identity, err := s.Application.UserService().DeprovisionUser(s.Ctx, userToDeprovision.Identity().Username)
identity, err := s.Application.UserService().BanUser(s.Ctx, userToBan.Identity().Username)
require.NoError(t, err)
assert.True(t, identity.User.Deprovisioned)
assert.Equal(t, userToDeprovision.User().ID, identity.User.ID)
assert.Equal(t, userToDeprovision.IdentityID(), identity.ID)
assert.True(t, identity.User.Banned)
assert.True(t, identity.User.Deprovisioned) // for backward compatibility
assert.Equal(t, userToBan.User().ID, identity.User.ID)
assert.Equal(t, userToBan.IdentityID(), identity.ID)

loadedUser := s.Graph.LoadUser(userToDeprovision.IdentityID())
assert.True(t, loadedUser.User().Deprovisioned)
userToDeprovision.Identity().User.Deprovisioned = true
testsupport.AssertIdentityEqual(t, userToDeprovision.Identity(), loadedUser.Identity())
loadedUser := s.Graph.LoadUser(userToBan.IdentityID())
assert.True(t, loadedUser.User().Banned)
userToBan.Identity().User.Banned = true
testsupport.AssertIdentityEqual(t, userToBan.Identity(), loadedUser.Identity())

loadedUser = s.Graph.LoadUser(userToStayIntact.IdentityID())
assert.Equal(t, false, loadedUser.User().Deprovisioned)
assert.Equal(t, false, loadedUser.User().Banned)
assert.Equal(t, false, loadedUser.User().Deprovisioned) // for backward compatibility
testsupport.AssertIdentityEqual(t, userToStayIntact.Identity(), loadedUser.Identity())
})

Expand All @@ -64,7 +63,7 @@ func (s *userServiceBlackboxTestSuite) TestDeprovision() {
// given
username := uuid.NewV4().String()
// when
_, err := s.Application.UserService().DeprovisionUser(s.Ctx, username)
_, err := s.Application.UserService().BanUser(s.Ctx, username)
// then
testsupport.AssertError(t, err, errors.NotFoundError{}, "user identity with username '%s' not found", username)

Expand Down Expand Up @@ -153,8 +152,8 @@ func (s *userServiceBlackboxTestSuite) TestDeactivate() {
identity, err := s.Application.UserService().DeactivateUser(ctx, userToDeactivate.Identity().Username)
// then
require.NoError(t, err)
assert.False(t, identity.User.Active) // user is inactive...
assert.False(t, identity.User.Deprovisioned) // ... but user NOT deprovisionned
assert.False(t, identity.User.Active) // user is inactive...
assert.False(t, identity.User.Banned) // ... but user NOT banned
assert.Equal(t, userToDeactivate.User().ID, identity.User.ID)
assert.Equal(t, userToDeactivate.IdentityID(), identity.ID)
// verify that user's fields were obfuscated and that the record was soft-deleted
Expand Down Expand Up @@ -232,26 +231,26 @@ func (s *userServiceBlackboxTestSuite) TestHardDeleteUser() {
})
}

func (s *userServiceBlackboxTestSuite) TestResetDeprovision() {
func (s *userServiceBlackboxTestSuite) TestResetBan() {
userToResetDeprovision := s.Graph.CreateUser()
userToStayIntact := s.Graph.CreateUser()

identity, err := s.Application.UserService().DeprovisionUser(s.Ctx, userToResetDeprovision.Identity().Username)
identity, err := s.Application.UserService().BanUser(s.Ctx, userToResetDeprovision.Identity().Username)
require.NoError(s.T(), err)
assert.True(s.T(), identity.User.Deprovisioned)
assert.True(s.T(), identity.User.Banned)

identityToStayIntact, err := s.Application.UserService().DeprovisionUser(s.Ctx, userToStayIntact.Identity().Username)
identityToStayIntact, err := s.Application.UserService().BanUser(s.Ctx, userToStayIntact.Identity().Username)
require.NoError(s.T(), err)
assert.True(s.T(), identityToStayIntact.User.Deprovisioned)
assert.True(s.T(), identityToStayIntact.User.Banned)

err = s.Application.UserService().ResetDeprovision(s.Ctx, identity.User)
err = s.Application.UserService().ResetBan(s.Ctx, identity.User)
require.NoError(s.T(), err)

loadedUser := s.Graph.LoadUser(userToResetDeprovision.IdentityID())
assert.False(s.T(), loadedUser.User().Deprovisioned)
assert.False(s.T(), loadedUser.User().Banned)

loadedUser = s.Graph.LoadUser(userToStayIntact.IdentityID())
assert.True(s.T(), loadedUser.User().Deprovisioned)
assert.True(s.T(), loadedUser.User().Banned)
}

func (s *userServiceBlackboxTestSuite) TestIdentityByUsernameAndEmail() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,11 @@ func (s *authenticationProviderServiceImpl) CreateOrUpdateIdentityAndUser(ctx co
return nil, nil, err
}

if identity.User.Deprovisioned {
if identity.User.Banned {
log.Warn(ctx, map[string]interface{}{
"identity_id": identity.ID,
"user_name": identity.Username,
}, "deprovisioned user tried to login")
}, "banned user tried to login")
return nil, nil, autherrors.NewUnauthorizedError("unauthorized access")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,12 +570,12 @@ func (s *authenticationProviderServiceTestSuite) checkAPIClientForUsersReturnOK(
s.checkLoginCallback(dummyIDPOAuthProviderRef, rw, authorizeCtx, "api_token")
}

func (s *authenticationProviderServiceTestSuite) TestDeprovisionedUserLoginUnauthorized() {
func (s *authenticationProviderServiceTestSuite) TestBannedUserLoginUnauthorized() {
extra := make(map[string]string)
_, callbackCtx := s.loginCallback(extra)

// Fails if identity is deprovisioned
identity, err := testsupport.CreateDeprovisionedTestIdentityAndUser(s.DB, "TestDeprovisionedUserLoginUnauthorized-"+uuid.NewV4().String())
// Fails if identity is banned
identity, err := testsupport.CreateBannedTestIdentityAndUser(s.DB, "TestBannedUserLoginUnauthorized-"+uuid.NewV4().String())
require.NoError(s.T(), err)

claims := make(map[string]interface{})
Expand All @@ -600,12 +600,12 @@ func (s *authenticationProviderServiceTestSuite) TestDeprovisionedUserLoginUnaut
require.IsType(s.T(), err, autherrors.UnauthorizedError{})
}

func (s *authenticationProviderServiceTestSuite) TestNotDeprovisionedUserLoginOK() {
func (s *authenticationProviderServiceTestSuite) TestNotBannedUserLoginOK() {
extra := make(map[string]string)
_, callbackCtx := s.loginCallback(extra)

// OK if identity is not deprovisioned
identity, err := testsupport.CreateTestIdentityAndUserWithDefaultProviderType(s.DB, "TestDeprovisionedUserLoginUnauthorized-"+uuid.NewV4().String())
// OK if identity is not banned
identity, err := testsupport.CreateTestIdentityAndUserWithDefaultProviderType(s.DB, "TestBannedUserLoginUnauthorized-"+uuid.NewV4().String())
require.NoError(s.T(), err)

claims := make(map[string]interface{})
Expand Down Expand Up @@ -796,12 +796,12 @@ func (s *authenticationProviderServiceTestSuite) TestExchangeRefreshTokenWithRPT
require.IsType(s.T(), autherrors.NewUnauthorizedError(""), err)
}

func (s *authenticationProviderServiceTestSuite) TestExchangeRefreshTokenFailsForDeprovisionedUser() {
func (s *authenticationProviderServiceTestSuite) TestExchangeRefreshTokenFailsForBannedUser() {
tm, err := manager.NewTokenManager(s.Configuration)
require.NoError(s.T(), err)

user := s.Graph.CreateUser()
user.Deprovision()
user.Ban()

ctx := manager.ContextWithTokenManager(testtoken.ContextWithRequest(nil), tm)
claims := make(map[string]interface{})
Expand Down
6 changes: 3 additions & 3 deletions authorization/invitation/service/invitation_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"github.com/fabric8-services/fabric8-auth/notification"
errs "github.com/pkg/errors"

"github.com/satori/go.uuid"
uuid "github.com/satori/go.uuid"
)

type InvitationConfiguration interface {
Expand Down Expand Up @@ -378,8 +378,8 @@ func (s *invitationServiceImpl) Accept(ctx context.Context, token uuid.UUID) (st
return "", redirectOnFailure, errs.Wrapf(err, "failed to load identity for invitee %d", currentIdentityID)
}

if identity.User.Deprovisioned {
return "", redirectOnFailure, autherrors.NewUnauthorizedError("user deprovisioned")
if identity.User.Banned {
return "", redirectOnFailure, autherrors.NewUnauthorizedError("user banned")
}

var resourceID string
Expand Down
4 changes: 2 additions & 2 deletions authorization/team/service/team_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ func (s *teamServiceImpl) CreateTeam(ctx context.Context, identityID uuid.UUID,
return errors.NewUnauthorizedError(fmt.Sprintf("unknown Identity ID %s", identityID))
}

if identity.User.Deprovisioned {
return errors.NewUnauthorizedError(fmt.Sprintf("user %s has been deprovisioned", identity.Username))
if identity.User.Banned {
return errors.NewUnauthorizedError(fmt.Sprintf("user %s has been banned", identity.Username))
}

// Validate the space resource
Expand Down
6 changes: 3 additions & 3 deletions authorization/token/manager/token_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ func (m *tokenManager) GenerateUnsignedUserAccessTokenFromClaims(ctx context.Con
claims["aud"] = tokenClaims.Audience
claims["typ"] = "Bearer"
claims["auth_time"] = tokenClaims.IssuedAt
claims["approved"] = identity != nil && !identity.User.Deprovisioned && tokenClaims.Approved
claims["approved"] = identity != nil && !identity.User.Banned && tokenClaims.Approved

if identity != nil {
claims["sub"] = identity.ID.String()
Expand Down Expand Up @@ -495,7 +495,7 @@ func (m *tokenManager) GenerateUnsignedUserAccessTokenForIdentity(ctx context.Co
claims["aud"] = openshiftIO
claims["typ"] = "Bearer"
claims["auth_time"] = iat // TODO should use the time when user actually logged-in the last time. Will need to get this time from the RHD token
claims["approved"] = !identity.User.Deprovisioned
claims["approved"] = !identity.User.Banned
claims["sub"] = identity.ID.String()
claims["email_verified"] = identity.User.EmailVerified
claims["name"] = identity.User.FullName
Expand Down Expand Up @@ -650,7 +650,7 @@ func (m *tokenManager) GenerateUnsignedUserAccessTokenFromRefreshToken(ctx conte
authOpenshiftIO,
openshiftIO,
}
claims["approved"] = identity != nil && !identity.User.Deprovisioned
claims["approved"] = identity != nil && !identity.User.Banned
if identity != nil {
claims["sub"] = identity.ID.String()
claims["email_verified"] = identity.User.EmailVerified
Expand Down
10 changes: 5 additions & 5 deletions authorization/token/manager/token_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ func (s *TestTokenSuite) TestGenerateUserTokenAndRefreshFlowForAPIClient() {
Username: username,
}

// we don't have any record in auth this is why deprovisioned is true to verify approved false.
identity.User.Deprovisioned = true
// we don't have any record in auth this is why banned is true to verify approved false.
identity.User.Banned = true
claims := make(map[string]interface{})
claims["sub"] = identityID.String()
claims["email"] = email
Expand All @@ -75,7 +75,7 @@ func (s *TestTokenSuite) TestGenerateUserTokenAndRefreshFlowForAPIClient() {
claims["given_name"] = username
claims["family_name"] = ""
claims["session_state"] = sessionState
claims["approved"] = !identity.User.Deprovisioned
claims["approved"] = !identity.User.Banned

accessToken, err := testtoken.GenerateAccessTokenWithClaims(claims)
if err != nil {
Expand Down Expand Up @@ -178,7 +178,7 @@ func (s *TestTokenSuite) checkGenerateRPTTokenForIdentity() {
s.assertClaim(rptClaims, "aud", "https://openshift.io")
s.assertClaim(rptClaims, "typ", "Bearer")
s.assertClaim(rptClaims, "auth_time", iat)
s.assertClaim(rptClaims, "approved", !identity.User.Deprovisioned)
s.assertClaim(rptClaims, "approved", !identity.User.Banned)
s.assertClaim(rptClaims, "sub", identity.ID.String())
s.assertClaim(rptClaims, "email", identity.User.Email)
s.assertClaim(rptClaims, "email_verified", identity.User.EmailVerified)
Expand Down Expand Up @@ -222,7 +222,7 @@ func (s *TestTokenSuite) assertGeneratedToken(generatedToken *oauth2.Token, iden
s.assertClaim(accessToken, "aud", "https://openshift.io")
s.assertClaim(accessToken, "typ", "Bearer")
s.assertClaim(accessToken, "auth_time", iat)
s.assertClaim(accessToken, "approved", !identity.User.Deprovisioned)
s.assertClaim(accessToken, "approved", !identity.User.Banned)
s.assertClaim(accessToken, "sub", identity.ID.String())
s.assertClaim(accessToken, "email", identity.User.Email)
s.assertClaim(accessToken, "email_verified", identity.User.EmailVerified)
Expand Down
Loading

0 comments on commit 4089f9c

Please sign in to comment.