Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v16] fix: Issue DeviceWebToken during Github authn #44808

Merged
merged 3 commits into from
Jul 30, 2024
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
3 changes: 3 additions & 0 deletions api/proto/teleport/legacy/types/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4755,6 +4755,9 @@ message GithubAuthRequest {
teleport.attestation.v1.AttestationStatement attestation_statement = 16 [(gogoproto.jsontag) = "attestation_statement,omitempty"];
// ClientLoginIP specifies IP address of the client for login, it will be written to the user's certificates.
string ClientLoginIP = 17 [(gogoproto.jsontag) = "client_login_ip,omitempty"];
// ClientUserAgent is the user agent of the Web browser, used for issuing
// a DeviceWebToken.
string ClientUserAgent = 18 [(gogoproto.jsontag) = "client_user_agent,omitempty"];
}

// SSOWarnings conveys a user-facing main message along with auxiliary warnings.
Expand Down
3,357 changes: 1,704 additions & 1,653 deletions api/types/types.pb.go
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Protogen conflicts solved by make grpc.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -4092,7 +4092,7 @@ func (a *Server) ExtendWebSession(ctx context.Context, req authclient.WebSession
}

sessionTTL := utils.ToTTL(a.clock, expiresAt)
sess, err := a.newWebSession(ctx, NewWebSessionRequest{
sess, _, err := a.newWebSession(ctx, NewWebSessionRequest{
User: req.User,
LoginIP: identity.LoginIP,
Roles: roles,
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3012,7 +3012,7 @@ func TestNewWebSession(t *testing.T) {
}
bearerTokenTTL := min(req.SessionTTL, defaults.BearerTokenTTL)

ws, err := p.a.newWebSession(ctx, req, nil /* opts */)
ws, _, err := p.a.newWebSession(ctx, req, nil /* opts */)
require.NoError(t, err)
require.Equal(t, user.GetName(), ws.GetUser())
require.Equal(t, duration, ws.GetIdleTimeout())
Expand Down
16 changes: 9 additions & 7 deletions lib/auth/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,13 +713,15 @@ func (a *Server) validateGithubAuthCallback(ctx context.Context, diagCtx *SSODia
// If the request is coming from a browser, create a web session.
if req.CreateWebSession {
session, err := a.CreateWebSessionFromReq(ctx, NewWebSessionRequest{
User: userState.GetName(),
Roles: userState.GetRoles(),
Traits: userState.GetTraits(),
SessionTTL: params.SessionTTL,
LoginTime: a.clock.Now().UTC(),
LoginIP: req.ClientLoginIP,
AttestWebSession: true,
User: userState.GetName(),
Roles: userState.GetRoles(),
Traits: userState.GetTraits(),
SessionTTL: params.SessionTTL,
LoginTime: a.clock.Now().UTC(),
LoginIP: req.ClientLoginIP,
LoginUserAgent: req.ClientUserAgent,
AttestWebSession: true,
CreateDeviceWebToken: true,
})
if err != nil {
return nil, trace.Wrap(err, "Failed to create web session.")
Expand Down
86 changes: 8 additions & 78 deletions lib/auth/methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,14 @@ import (

"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/constants"
devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1"
"github.com/gravitational/teleport/api/types"
apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/lib/auth/authclient"
wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes"
"github.com/gravitational/teleport/lib/authz"
"github.com/gravitational/teleport/lib/defaults"
dtconfig "github.com/gravitational/teleport/lib/devicetrust/config"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/utils"
)
Expand Down Expand Up @@ -630,89 +627,22 @@ func (a *Server) AuthenticateWebUser(ctx context.Context, req authclient.Authent
}

sess, err := a.CreateWebSessionFromReq(ctx, NewWebSessionRequest{
User: user.GetName(),
LoginIP: loginIP,
Roles: user.GetRoles(),
Traits: user.GetTraits(),
LoginTime: a.clock.Now().UTC(),
AttestWebSession: true,
User: user.GetName(),
LoginIP: loginIP,
LoginUserAgent: userAgent,
Roles: user.GetRoles(),
Traits: user.GetTraits(),
LoginTime: a.clock.Now().UTC(),
AttestWebSession: true,
CreateDeviceWebToken: true,
})
if err != nil {
return nil, trace.Wrap(err)
}

// Create the device trust DeviceWebToken.
// We only get a token if the server is enabled for Device Trust and the user
// has a suitable trusted device.
if loginIP != "" && userAgent != "" {
webToken, err := a.createDeviceWebToken(ctx, &devicepb.DeviceWebToken{
WebSessionId: sess.GetName(),
BrowserUserAgent: userAgent,
BrowserIp: loginIP,
User: sess.GetUser(),
})
switch {
case err != nil:
log.WithError(err).Warn("Failed to create DeviceWebToken for user")
case webToken != nil: // May be nil even if err==nil.
sess.SetDeviceWebToken(&types.DeviceWebToken{
Id: webToken.Id,
Token: webToken.Token,
})
}
}

// Calculate the trusted device requirement for the session. Helps inform the
// frontend if the user might run into access problems without a trusted
// device.
trustedDeviceRequirement, err := a.calculateTrustedDeviceMode(ctx, func() ([]types.Role, error) {
// TODO(codingllama): Levegare the checker inside CreateWebSessionFromReq to
// avoid duplicate work here.
roles, err := services.FetchRoles(user.GetRoles(), a, user.GetTraits())
if err != nil {
return nil, trace.Wrap(err)
}
return roles, nil
})
if err != nil {
return nil, trace.Wrap(err)
}
sess.SetTrustedDeviceRequirement(trustedDeviceRequirement)

return sess, nil
}

func (a *Server) calculateTrustedDeviceMode(ctx context.Context, getRoles func() ([]types.Role, error)) (types.TrustedDeviceRequirement, error) {
const unspecified = types.TrustedDeviceRequirement_TRUSTED_DEVICE_REQUIREMENT_UNSPECIFIED

// Don't evaluate for OSS.
if !modules.GetModules().IsEnterpriseBuild() {
return unspecified, nil
}

// Required by cluster mode?
ap, err := a.GetAuthPreference(ctx)
if err != nil {
return unspecified, trace.Wrap(err)
}
if dtconfig.GetEffectiveMode(ap.GetDeviceTrust()) == constants.DeviceTrustModeRequired {
return types.TrustedDeviceRequirement_TRUSTED_DEVICE_REQUIREMENT_REQUIRED, nil
}

// Required by roles?
roles, err := getRoles()
if err != nil {
return unspecified, trace.Wrap(err)
}
for _, role := range roles {
if role.GetOptions().DeviceTrustMode == constants.DeviceTrustModeRequired {
return types.TrustedDeviceRequirement_TRUSTED_DEVICE_REQUIREMENT_REQUIRED, nil
}
}

return types.TrustedDeviceRequirement_TRUSTED_DEVICE_REQUIREMENT_NOT_REQUIRED, nil
}

// AuthenticateSSHUser authenticates an SSH user and returns SSH and TLS
// certificates for the public key in req.
func (a *Server) AuthenticateSSHUser(ctx context.Context, req authclient.AuthenticateSSHRequest) (*authclient.SSHLoginResponse, error) {
Expand Down
Loading
Loading