Skip to content

Commit

Permalink
Only proxy role can get/create SAML IdP sessions. (#41845)
Browse files Browse the repository at this point in the history
  • Loading branch information
Joerger committed May 21, 2024
1 parent d1785ff commit d150568
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 23 deletions.
16 changes: 6 additions & 10 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -5410,19 +5410,17 @@ func (a *ServerWithRoles) GetSnowflakeSession(ctx context.Context, req types.Get

// GetSAMLIdPSession gets a SAML IdP session.
func (a *ServerWithRoles) GetSAMLIdPSession(ctx context.Context, req types.GetSAMLIdPSessionRequest) (types.WebSession, error) {
if err := a.action(apidefaults.Namespace, types.KindWebSession, types.VerbRead); err != nil {
return nil, trace.Wrap(err)
}

session, err := a.authServer.GetSAMLIdPSession(ctx, req)
if err != nil {
return nil, trace.Wrap(err)
}
if session.GetSubKind() != types.KindSAMLIdPSession {
return nil, trace.AccessDenied("GetSAMLIdPSession only allows reading sessions with SubKind SAMLIdpSession")
}
// Users can only fetch their own web sessions or the proxy can fetch all web sessions.
if err := a.currentUserAction(session.GetUser()); err != nil {
if err := a.action(apidefaults.Namespace, types.KindWebSession, types.VerbRead); err != nil {
return nil, trace.Wrap(err)
}
}
return session, nil
}

Expand Down Expand Up @@ -5497,12 +5495,10 @@ func (a *ServerWithRoles) CreateSnowflakeSession(ctx context.Context, req types.
func (a *ServerWithRoles) CreateSAMLIdPSession(ctx context.Context, req types.CreateSAMLIdPSessionRequest) (types.WebSession, error) {
// Check if this a proxy service.
if !a.hasBuiltinRole(types.RoleProxy) {
if err := a.currentUserAction(req.Username); err != nil {
return nil, trace.Wrap(err)
}
return nil, trace.AccessDenied("this request can be only executed by a proxy")
}

samlSession, err := a.authServer.CreateSAMLIdPSession(ctx, req, a.context.Identity.GetIdentity(), a.context.Checker)
samlSession, err := a.authServer.CreateSAMLIdPSession(ctx, req)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
15 changes: 5 additions & 10 deletions lib/auth/auth_with_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6863,15 +6863,15 @@ func TestCreateSAMLIdPSession(t *testing.T) {
},
"as session user": {
identity: TestUser(alice),
assertErr: require.NoError,
assertErr: require.Error,
},
"as other user": {
identity: TestUser(bob),
assertErr: require.Error,
},
"as admin user": {
identity: TestUser(admin),
assertErr: require.NoError,
assertErr: require.Error,
},
}
for name, test := range tests {
Expand Down Expand Up @@ -6899,11 +6899,8 @@ func TestGetSAMLIdPSession(t *testing.T) {

ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
// setup a session to get, for user "alice".
aliceClient, err := srv.NewClient(TestUser(alice))
require.NoError(t, err)

sess, err := aliceClient.CreateSAMLIdPSession(ctx, types.CreateSAMLIdPSessionRequest{
sess, err := srv.Auth().CreateSAMLIdPSession(ctx, types.CreateSAMLIdPSessionRequest{
SessionID: "test",
Username: alice,
SAMLSession: &types.SAMLSessionData{},
Expand All @@ -6920,7 +6917,7 @@ func TestGetSAMLIdPSession(t *testing.T) {
},
"as session user": {
identity: TestUser(alice),
assertErr: require.NoError,
assertErr: require.Error,
},
"as other user": {
identity: TestUser(bob),
Expand Down Expand Up @@ -7011,15 +7008,13 @@ func TestDeleteSAMLIdPSession(t *testing.T) {
},
}

aliceClient, err := srv.NewClient(TestUser(alice))
require.NoError(t, err)
for name, test := range tests {
test := test
t.Run(name, func(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
sess, err := aliceClient.CreateSAMLIdPSession(ctx, types.CreateSAMLIdPSessionRequest{
sess, err := srv.Auth().CreateSAMLIdPSession(ctx, types.CreateSAMLIdPSessionRequest{
SessionID: uuid.NewString(),
Username: alice,
SAMLSession: &types.SAMLSessionData{},
Expand Down
4 changes: 1 addition & 3 deletions lib/auth/sessions.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,9 +538,7 @@ func (a *Server) CreateSnowflakeSession(ctx context.Context, req types.CreateSno
return session, nil
}

func (a *Server) CreateSAMLIdPSession(ctx context.Context, req types.CreateSAMLIdPSessionRequest,
identity tlsca.Identity, checker services.AccessChecker,
) (types.WebSession, error) {
func (a *Server) CreateSAMLIdPSession(ctx context.Context, req types.CreateSAMLIdPSessionRequest) (types.WebSession, error) {
// TODO(mdwn): implement a module.Features() check.

if req.SAMLSession == nil {
Expand Down

0 comments on commit d150568

Please sign in to comment.