From d42c896b939fdaa2cbfd8c83f4a0584e17505978 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Thu, 27 Jul 2023 17:56:31 +0000 Subject: [PATCH 01/24] Constructed Okta login auth redirect after accepting eula terms. This is the product of pair programming and is being shared for team development purposes. There is still much to be done and cleaned up on. --- .gitignore | 1 + cmd/milmove/serve.go | 7 +- pkg/handlers/authentication/auth.go | 16 +++-- pkg/handlers/authentication/okta.go | 79 ++++++++++++++++++++++ pkg/handlers/routing/base_routing_suite.go | 5 +- 5 files changed, 96 insertions(+), 12 deletions(-) create mode 100644 pkg/handlers/authentication/okta.go diff --git a/.gitignore b/.gitignore index d7d0bd13e66..a7bbf7cbd3e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ # direnv .envrc.local .envrc.chamber +.envrc.okta # dependencies /vendor diff --git a/cmd/milmove/serve.go b/cmd/milmove/serve.go index 5f134e09bd8..21b84ea01ab 100644 --- a/cmd/milmove/serve.go +++ b/cmd/milmove/serve.go @@ -484,14 +484,15 @@ func buildRoutingConfig(appCtx appcontext.AppContext, v *viper.Viper, redisPool appCtx.Logger().Fatal("Must provide the Login.gov hostname parameter, exiting") } - // Register Login.gov authentication provider for My.(move.mil) - loginGovProvider, err := authentication.InitAuth(v, appCtx.Logger(), + // Register Okta authentication provider for My.(move.mil) + oktaProvider, err := authentication.InitAuth(v, appCtx.Logger(), appNames) if err != nil { appCtx.Logger().Fatal("Registering login provider", zap.Error(err)) } - routingConfig.AuthContext = authentication.NewAuthContext(appCtx.Logger(), loginGovProvider, loginGovCallbackProtocol, loginGovCallbackPort) + // TODO: Update loginGov callbacks to Okta + routingConfig.AuthContext = authentication.NewAuthContext(appCtx.Logger(), *oktaProvider, loginGovCallbackProtocol, loginGovCallbackPort) // Email notificationSender, err := notifications.InitEmail(v, awsSession, appCtx.Logger()) diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index 89dc3464bfc..3e324b9e485 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -457,8 +457,10 @@ func (context *Context) GetFeatureFlag(flag string) bool { } // Context is the common handler type for auth handlers +// TODO: Remove loginGov type Context struct { loginGovProvider LoginGovProvider + oktaProvider OktaProvider callbackTemplate string featureFlags map[string]bool } @@ -470,9 +472,9 @@ type FeatureFlag struct { } // NewAuthContext creates an Context -func NewAuthContext(_ *zap.Logger, loginGovProvider LoginGovProvider, callbackProtocol string, callbackPort int) Context { +func NewAuthContext(_ *zap.Logger, oktaProvider OktaProvider, callbackProtocol string, callbackPort int) Context { context := Context{ - loginGovProvider: loginGovProvider, + oktaProvider: oktaProvider, callbackTemplate: fmt.Sprintf("%s://%%s:%d/", callbackProtocol, callbackPort), } return context @@ -584,6 +586,7 @@ func StateCookieName(session *auth.Session) string { } // RedirectHandler constructs the Login.gov authentication URL and redirects to it +// TODO: More login.gov to Okta func (h RedirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx := h.AppContextFromRequest(r) @@ -593,7 +596,7 @@ func (h RedirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - loginData, err := h.loginGovProvider.AuthorizationURL(r) + loginData, err := h.oktaProvider.AuthorizationURL(r) if err != nil { http.Error(w, http.StatusText(500), http.StatusInternalServerError) return @@ -1139,14 +1142,15 @@ func fetchToken(code string, clientID string, loginGovProvider LoginGovProvider) return &session, err } -// InitAuth initializes the Login.gov provider -func InitAuth(v *viper.Viper, logger *zap.Logger, appnames auth.ApplicationServername) (LoginGovProvider, error) { +// InitAuth initializes the Okta provider +func InitAuth(v *viper.Viper, logger *zap.Logger, appnames auth.ApplicationServername) (*OktaProvider, error) { loginGovCallbackProtocol := v.GetString(cli.LoginGovCallbackProtocolFlag) loginGovCallbackPort := v.GetInt(cli.LoginGovCallbackPortFlag) loginGovSecretKey := v.GetString(cli.LoginGovSecretKeyFlag) loginGovHostname := v.GetString(cli.LoginGovHostnameFlag) loginGovProvider := NewLoginGovProvider(loginGovHostname, loginGovSecretKey, logger) + oktaProvider := NewOktaProvider(logger) err := loginGovProvider.RegisterProvider( appnames.MilServername, v.GetString(cli.LoginGovMyClientIDFlag), @@ -1156,5 +1160,5 @@ func InitAuth(v *viper.Viper, logger *zap.Logger, appnames auth.ApplicationServe v.GetString(cli.LoginGovAdminClientIDFlag), loginGovCallbackProtocol, loginGovCallbackPort) - return loginGovProvider, err + return oktaProvider, err } diff --git a/pkg/handlers/authentication/okta.go b/pkg/handlers/authentication/okta.go new file mode 100644 index 00000000000..299d892d8c8 --- /dev/null +++ b/pkg/handlers/authentication/okta.go @@ -0,0 +1,79 @@ +package authentication + +import ( + "net/http" + "net/url" + "os" + + "github.com/markbates/goth/providers/okta" + "github.com/transcom/mymove/pkg/auth" + "go.uber.org/zap" +) + +type OktaProvider struct { + okta.Provider + Logger *zap.Logger +} + +type OktaData struct { + RedirectURL string + Nonce string +} + +func (op *OktaProvider) AuthorizationURL(r *http.Request) (*OktaData, error) { + // if os.Getenv("OKTA_OAUTH2_ISSUER") == "" { + // err := errors.New("Issuer not set") + // op.logger.Error("Issuer not set", zap.Error(err)) + // return nil, err + // } + + state := generateNonce() + + sess, err := op.BeginAuth(state) + if err != nil { + op.Logger.Error("Goth begin auth", zap.Error(err)) + return nil, err + } + + baseURL, err := sess.GetAuthURL() + if err != nil { + op.Logger.Error("Goth get auth URL", zap.Error(err)) + return nil, err + } + + authURL, err := url.Parse(baseURL) + if err != nil { + op.Logger.Error("Parse auth URL", zap.Error(err)) + return nil, err + } + + // TODO: Verify CAC authenticator + params := authURL.Query() + session := auth.SessionFromRequestContext(r) + // TODO: Switch away from idmanagement - This is login.gov + if session.IsAdminApp() { + // This specifies that a user has been authenticated with an HSPD12 credential, via their CAC. Both acr_values must be specified. + params.Add("acr_values", "http://idmanagement.gov/ns/assurance/ial/1 http://idmanagement.gov/ns/assurance/aal/3?hspd12=true") + } else { + params.Add("acr_values", "http://idmanagement.gov/ns/assurance/loa/1") + } + params.Add("nonce", state) + params.Set("scope", "openid email") + + authURL.RawQuery = params.Encode() + + return &OktaData{authURL.String(), state}, nil +} + +func NewOktaProvider(logger *zap.Logger) *OktaProvider { + return &OktaProvider{ + Provider: *okta.New( + os.Getenv("OKTA_OAUTH2_CLIENT_ID"), + os.Getenv("OKTA_OAUTH2_CLIENT_SECRET"), + os.Getenv("OKTA_OAUTH2_ISSUER"), + "http://milmovelocal:3000/", + "openid", "profile", "email", + ), + Logger: logger, + } +} diff --git a/pkg/handlers/routing/base_routing_suite.go b/pkg/handlers/routing/base_routing_suite.go index 5fcdbae9194..1a103de564b 100644 --- a/pkg/handlers/routing/base_routing_suite.go +++ b/pkg/handlers/routing/base_routing_suite.go @@ -60,9 +60,8 @@ func (suite *BaseRoutingSuite) RoutingConfig() *Config { fakeS3 := storageTest.NewFakeS3Storage(true) handlerConfig.SetFileStorer(fakeS3) - fakeLoginGovProvider := authentication.NewLoginGovProvider("fakeHostname", "secret_key", suite.Logger()) - - authContext := authentication.NewAuthContext(suite.Logger(), fakeLoginGovProvider, "http", 80) + fakeOktaProvider := authentication.NewOktaProvider(suite.Logger()) + authContext := authentication.NewAuthContext(suite.Logger(), *fakeOktaProvider, "http", 80) fakeFs := afero.NewMemMapFs() fakeBase := "fakebase" From aca65d20aa274ba7688419a3fb59d5fc0ba42263 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Wed, 2 Aug 2023 19:23:25 +0000 Subject: [PATCH 02/24] Began registration and key management of three additional auth providers for Okta --- pkg/cli/auth.go | 48 +++++++++++++++++++++++++++++ pkg/handlers/authentication/auth.go | 30 +++++++++--------- pkg/handlers/authentication/okta.go | 44 ++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 14 deletions(-) diff --git a/pkg/cli/auth.go b/pkg/cli/auth.go index 14ff3f894ee..f02a54c6363 100644 --- a/pkg/cli/auth.go +++ b/pkg/cli/auth.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "os" "regexp" "strings" @@ -21,6 +22,10 @@ const ( //RA Validator: jneuner@mitre.org //RA Modified Severity: CAT III // #nosec G101 + // + // Additional notes for Okta flags - these variable names with "secret" are to identify the name of the flag as well + // These variables do not store the secret + // ClientAuthSecretKeyFlag is the Client Auth Secret Key Flag ClientAuthSecretKeyFlag string = "client-auth-secret-key" // LoginGovCallbackProtocolFlag is the Login.gov Callback Protocol Flag @@ -37,6 +42,25 @@ const ( LoginGovAdminClientIDFlag string = "login-gov-admin-client-id" // LoginGovHostnameFlag is the Login.gov Hostname Flag LoginGovHostnameFlag string = "login-gov-hostname" + + // Okta tenant flags + OktaTenantIssuerURLFlag string = "okta-tenant-issuer-url" + OktaTenantCallbackPortFlag string = "okta-tenant-callback-port" + // Okta Customer client id and secret flags + OktaCustomerSecretKeyFlag string = "okta-customer-secret-key" + OktaCustomerClientIDFlag string = "okta-customr-client-id" + OktaCustomerHostnameFlag string = "okta-customer-hostname" + OktaCustomerCallbackProtocolFlag string = "okta-customer-callback-protocol" + // Okta Office client id and secret flags + OktaOfficeSecretKeyFlag string = "okta-office-secret-key" + OktaOfficeClientIDFlag string = "okta-office-client-id" + OktaOfficeHostnameFlag string = "okta-office-hostname" + OktaOfficeCallbackProtocolFlag string = "okta-office-callback-protocol" + // Okta Admin client id and secret flags + OktaAdminSecretKeyFlag string = "okta-admin-secret-key" + OktaAdminClientIDFlag string = "okta-admin-client-id" + OktaAdminHostnameFlag string = "okta-admin-hostname" + OktaAdminCallbackProtocolFlag string = "okta-admin-callback-protocol" ) type errInvalidClientID struct { @@ -58,6 +82,30 @@ func InitAuthFlags(flag *pflag.FlagSet) { flag.String(LoginGovOfficeClientIDFlag, "", "Client ID registered with login gov.") flag.String(LoginGovAdminClientIDFlag, "", "Client ID registered with login gov.") flag.String(LoginGovHostnameFlag, "secure.login.gov", "Hostname for communicating with login gov.") + + // TODO: Replace Okta os.Getenv + + // Okta flags + flag.String(OktaTenantIssuerURLFlag, os.Getenv("OKTA_TENANT_ISSUER_URL"), "Okta tenant issuer URL.") + flag.Int(OktaTenantCallbackPortFlag, 443, "Okta tenant callback port.") + + // Customer flags + flag.String(OktaCustomerSecretKeyFlag, os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), "Okta customer secret key.") + flag.String(OktaCustomerClientIDFlag, os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), "Okta customer client ID.") + flag.String(OktaCustomerHostnameFlag, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), "Okta customer hostname.") + flag.String(OktaCustomerCallbackProtocolFlag, os.Getenv("OKTA_CUSTOMER_CALLBACK_PROTOCOL"), "Okta customer callback protocol.") + + // Office flags + flag.String(OktaOfficeSecretKeyFlag, os.Getenv("OKTA_OFFICE_SECRET_KEY"), "Okta office secret key.") + flag.String(OktaOfficeClientIDFlag, os.Getenv("OKTA_OFFICE_CLIENT_ID"), "Okta office client ID.") + flag.String(OktaOfficeHostnameFlag, os.Getenv("OKTA_OFFICE_HOSTNAME"), "Okta office hostname.") + flag.String(OktaOfficeCallbackProtocolFlag, os.Getenv("OKTA_OFFICE_CALLBACK_PROTOCOL"), "Okta office callback protocol.") + + // Admin flags + flag.String(OktaAdminSecretKeyFlag, os.Getenv("OKTA_ADMIN_SECRET_KEY"), "Okta admin secret key.") + flag.String(OktaAdminClientIDFlag, os.Getenv("OKTA_ADMIN_CLIENT_ID"), "Okta admin client ID.") + flag.String(OktaAdminHostnameFlag, os.Getenv("OKTA_ADMIN_HOSTNAME"), "Okta admin hostname.") + flag.String(OktaAdminCallbackProtocolFlag, os.Getenv("OKTA_ADMIN_CALLBACK_PROTOCOL"), "Okta admin callback protocol.") } // CheckAuth validates Auth command line flags diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index 3e324b9e485..5d807ec6922 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -1142,23 +1142,25 @@ func fetchToken(code string, clientID string, loginGovProvider LoginGovProvider) return &session, err } -// InitAuth initializes the Okta provider +// InitAuth initializes the Okta and Logingov provider func InitAuth(v *viper.Viper, logger *zap.Logger, appnames auth.ApplicationServername) (*OktaProvider, error) { - loginGovCallbackProtocol := v.GetString(cli.LoginGovCallbackProtocolFlag) - loginGovCallbackPort := v.GetInt(cli.LoginGovCallbackPortFlag) - loginGovSecretKey := v.GetString(cli.LoginGovSecretKeyFlag) - loginGovHostname := v.GetString(cli.LoginGovHostnameFlag) - loginGovProvider := NewLoginGovProvider(loginGovHostname, loginGovSecretKey, logger) oktaProvider := NewOktaProvider(logger) - err := loginGovProvider.RegisterProvider( - appnames.MilServername, - v.GetString(cli.LoginGovMyClientIDFlag), - appnames.OfficeServername, + err := oktaProvider.RegisterProviders( + v.GetString(cli.OktaCustomerHostnameFlag), + v.GetString(cli.OktaTenantIssuerURLFlag), + v.GetString(cli.OktaCustomerClientIDFlag), + v.GetString(cli.OktaCustomerSecretKeyFlag), + v.GetString(cli.OktaCustomerHostnameFlag), + v.GetString(cli.OktaTenantIssuerURLFlag), + v.GetString(cli.OktaOfficeClientIDFlag), + v.GetString(cli.OktaOfficeSecretKeyFlag), + v.GetString(cli.OktaOfficeHostnameFlag), + v.GetString(cli.OktaTenantIssuerURLFlag), + v.GetString(cli.OktaAdminClientIDFlag), + v.GetString(cli.OktaAdminSecretKeyFlag), v.GetString(cli.LoginGovOfficeClientIDFlag), - appnames.AdminServername, - v.GetString(cli.LoginGovAdminClientIDFlag), - loginGovCallbackProtocol, - loginGovCallbackPort) + v.GetInt(cli.OktaTenantCallbackPortFlag), + v.GetString(cli.OktaTenantIssuerURLFlag)) return oktaProvider, err } diff --git a/pkg/handlers/authentication/okta.go b/pkg/handlers/authentication/okta.go index 299d892d8c8..6023f4cffca 100644 --- a/pkg/handlers/authentication/okta.go +++ b/pkg/handlers/authentication/okta.go @@ -5,11 +5,17 @@ import ( "net/url" "os" + "github.com/markbates/goth" "github.com/markbates/goth/providers/okta" "github.com/transcom/mymove/pkg/auth" "go.uber.org/zap" ) +const customerProviderName = "customerProvider" + +// const officeProviderName = "officeProvider" //used in the login_gov.go +// const adminProviderName = "adminProvider" // used in login_gov.go + type OktaProvider struct { okta.Provider Logger *zap.Logger @@ -77,3 +83,41 @@ func NewOktaProvider(logger *zap.Logger) *OktaProvider { Logger: logger, } } + +// This function allows us to wrap new registered providers with the zap logger. The initial Okta provider is already wrapped +func wrapOktaProvider(provider *okta.Provider, logger *zap.Logger) *OktaProvider { + return &OktaProvider{ + Provider: *provider, + Logger: logger, + } +} + +// This function allows us to register a new Okta provider with Goth. This is primarily used +// for the three different Okta applications we're supporting: Customer, Office, and Admin +func (op *OktaProvider) RegisterProvider(providerName string, clientID string, clientSecret string, issuerURL string, callbackURL string) error { + + oktaProvider := okta.NewCustomisedURL(clientID, clientSecret, callbackURL, issuerURL+"/v1/authorize", issuerURL+"/v1/token", issuerURL, issuerURL+"/v1/userinfo", "openid", "profile", "email") + + // set provider name for the Okta provider + oktaProvider.SetName(providerName) + + goth.UseProviders( + wrapOktaProvider(oktaProvider, op.Logger), + ) + + return nil +} + +func (op *OktaProvider) RegisterProviders(customerHostname string, customerCallbackUrl string, customerClientID string, customerSecret string, officeHostname string, officeCallbackUrl string, officeClientID string, officeSecret string, adminHostname string, adminCallbackUrl string, adminClientID string, adminSecret string, callbackProtocol string, callbackPort int, oktaIssuer string) error { + customerProvider := okta.New(customerClientID, customerSecret, oktaIssuer, customerCallbackUrl, "openid", "profile", "email") + officeProvider := okta.New(officeClientID, officeSecret, oktaIssuer, officeCallbackUrl, "openid", "profile", "email") + adminProvider := okta.New(adminClientID, adminSecret, oktaIssuer, adminCallbackUrl, "openid", "profile", "email") + + goth.UseProviders( + wrapOktaProvider(customerProvider, op.Logger), + wrapOktaProvider(officeProvider, op.Logger), + wrapOktaProvider(adminProvider, op.Logger), + ) + + return nil +} From 4f80af9a60e0187d2199538ce241934225e99a3a Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Fri, 4 Aug 2023 12:27:54 +0000 Subject: [PATCH 03/24] Completed initial functionality of okta providers for all three subdomains. Login redirect is working as intended. Modified gitignore for better vscode debug support --- .gitignore | 1 + pkg/handlers/authentication/auth.go | 54 +++++++++++++++++------- pkg/handlers/authentication/okta.go | 65 +++++++++++++++-------------- 3 files changed, 72 insertions(+), 48 deletions(-) diff --git a/.gitignore b/.gitignore index a7bbf7cbd3e..47495393f7a 100644 --- a/.gitignore +++ b/.gitignore @@ -30,6 +30,7 @@ bin/ #debugging /cmd/milmove/debug /cmd/webserver/debug +__debug_bin* # misc .DS_Store diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index 5d807ec6922..fc4c003dce7 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -9,6 +9,7 @@ import ( "io" "net/http" "net/url" + "os" "strings" "time" @@ -22,7 +23,6 @@ import ( "github.com/transcom/mymove/pkg/appcontext" "github.com/transcom/mymove/pkg/auth" - "github.com/transcom/mymove/pkg/cli" "github.com/transcom/mymove/pkg/handlers" "github.com/transcom/mymove/pkg/logging" "github.com/transcom/mymove/pkg/models" @@ -1145,22 +1145,44 @@ func fetchToken(code string, clientID string, loginGovProvider LoginGovProvider) // InitAuth initializes the Okta and Logingov provider func InitAuth(v *viper.Viper, logger *zap.Logger, appnames auth.ApplicationServername) (*OktaProvider, error) { + // Create a new Okta Provider. This will be used in the creation of the additional providers for each subdomain oktaProvider := NewOktaProvider(logger) + /* + err := oktaProvider.RegisterProviders( + v.GetString(cli.OktaCustomerHostnameFlag), + v.GetString(cli.OktaTenantIssuerURLFlag), + v.GetString(cli.OktaCustomerClientIDFlag), + v.GetString(cli.OktaCustomerSecretKeyFlag), + v.GetString(cli.OktaCustomerHostnameFlag), + v.GetString(cli.OktaTenantIssuerURLFlag), + v.GetString(cli.OktaOfficeClientIDFlag), + v.GetString(cli.OktaOfficeSecretKeyFlag), + v.GetString(cli.OktaOfficeHostnameFlag), + v.GetString(cli.OktaTenantIssuerURLFlag), + v.GetString(cli.OktaAdminClientIDFlag), + v.GetString(cli.OktaAdminSecretKeyFlag), + v.GetString(cli.LoginGovOfficeClientIDFlag), + v.GetInt(cli.OktaTenantCallbackPortFlag), + v.GetString(cli.OktaTenantIssuerURLFlag)) + */ + + // TODO: Repace temporary envrc values once we get chamber access err := oktaProvider.RegisterProviders( - v.GetString(cli.OktaCustomerHostnameFlag), - v.GetString(cli.OktaTenantIssuerURLFlag), - v.GetString(cli.OktaCustomerClientIDFlag), - v.GetString(cli.OktaCustomerSecretKeyFlag), - v.GetString(cli.OktaCustomerHostnameFlag), - v.GetString(cli.OktaTenantIssuerURLFlag), - v.GetString(cli.OktaOfficeClientIDFlag), - v.GetString(cli.OktaOfficeSecretKeyFlag), - v.GetString(cli.OktaOfficeHostnameFlag), - v.GetString(cli.OktaTenantIssuerURLFlag), - v.GetString(cli.OktaAdminClientIDFlag), - v.GetString(cli.OktaAdminSecretKeyFlag), - v.GetString(cli.LoginGovOfficeClientIDFlag), - v.GetInt(cli.OktaTenantCallbackPortFlag), - v.GetString(cli.OktaTenantIssuerURLFlag)) + os.Getenv("OKTA_CUSTOMER_HOSTNAME"), + "http://milmovelocal:3000/", + os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), + os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), + os.Getenv("OKTA_OFFICE_HOSTNAME"), + "http://officelocal:3000/", + os.Getenv("OKTA_OFFICE_CLIENT_ID"), + os.Getenv("OKTA_OFFICE_SECRET_KEY"), + os.Getenv("OKTA_ADMIN_HOSTNAME"), + "http://adminlocal:3000/", + os.Getenv("OKTA_ADMIN_CLIENT_ID"), + os.Getenv("OKTA_ADMIN_SECRET_KEY"), + "https", + 443, + os.Getenv("OKTA_TENANT_ISSUER_URL"), + ) return oktaProvider, err } diff --git a/pkg/handlers/authentication/okta.go b/pkg/handlers/authentication/okta.go index 6023f4cffca..ffba49d9591 100644 --- a/pkg/handlers/authentication/okta.go +++ b/pkg/handlers/authentication/okta.go @@ -26,16 +26,37 @@ type OktaData struct { Nonce string } +// This function will select the correct provider to use based on its set name. +func getOktaProviderForRequest(r *http.Request, oktaProvider OktaProvider) (goth.Provider, error) { + session := auth.SessionFromRequestContext(r) + providerName := customerProviderName + if session.IsOfficeApp() { + providerName = officeProviderName + } else if session.IsAdminApp() { + providerName = adminProviderName + } + gothProvider, err := goth.GetProvider(providerName) + if err != nil { + return nil, err + } + + return gothProvider, nil +} + +// This function will use the OktaProvider to return the correct authorization URL to use func (op *OktaProvider) AuthorizationURL(r *http.Request) (*OktaData, error) { - // if os.Getenv("OKTA_OAUTH2_ISSUER") == "" { - // err := errors.New("Issuer not set") - // op.logger.Error("Issuer not set", zap.Error(err)) - // return nil, err - // } + + // Retrieve the correct Okta Provider to use to get the correct authorization URL. This will choose from customer, + // office, or admin domains. + provider, err := getOktaProviderForRequest(r, *op) + if err != nil { + op.Logger.Error("Get Goth provider", zap.Error(err)) + return nil, err + } state := generateNonce() - sess, err := op.BeginAuth(state) + sess, err := provider.BeginAuth(state) if err != nil { op.Logger.Error("Goth begin auth", zap.Error(err)) return nil, err @@ -53,16 +74,7 @@ func (op *OktaProvider) AuthorizationURL(r *http.Request) (*OktaData, error) { return nil, err } - // TODO: Verify CAC authenticator params := authURL.Query() - session := auth.SessionFromRequestContext(r) - // TODO: Switch away from idmanagement - This is login.gov - if session.IsAdminApp() { - // This specifies that a user has been authenticated with an HSPD12 credential, via their CAC. Both acr_values must be specified. - params.Add("acr_values", "http://idmanagement.gov/ns/assurance/ial/1 http://idmanagement.gov/ns/assurance/aal/3?hspd12=true") - } else { - params.Add("acr_values", "http://idmanagement.gov/ns/assurance/loa/1") - } params.Add("nonce", state) params.Set("scope", "openid email") @@ -71,6 +83,7 @@ func (op *OktaProvider) AuthorizationURL(r *http.Request) (*OktaData, error) { return &OktaData{authURL.String(), state}, nil } +// TODO: Clean up when working on callback func NewOktaProvider(logger *zap.Logger) *OktaProvider { return &OktaProvider{ Provider: *okta.New( @@ -92,27 +105,15 @@ func wrapOktaProvider(provider *okta.Provider, logger *zap.Logger) *OktaProvider } } -// This function allows us to register a new Okta provider with Goth. This is primarily used -// for the three different Okta applications we're supporting: Customer, Office, and Admin -func (op *OktaProvider) RegisterProvider(providerName string, clientID string, clientSecret string, issuerURL string, callbackURL string) error { - - oktaProvider := okta.NewCustomisedURL(clientID, clientSecret, callbackURL, issuerURL+"/v1/authorize", issuerURL+"/v1/token", issuerURL, issuerURL+"/v1/userinfo", "openid", "profile", "email") - - // set provider name for the Okta provider - oktaProvider.SetName(providerName) - - goth.UseProviders( - wrapOktaProvider(oktaProvider, op.Logger), - ) - - return nil -} - +// Function to register all three providers at once. +// TODO: Split this function up func (op *OktaProvider) RegisterProviders(customerHostname string, customerCallbackUrl string, customerClientID string, customerSecret string, officeHostname string, officeCallbackUrl string, officeClientID string, officeSecret string, adminHostname string, adminCallbackUrl string, adminClientID string, adminSecret string, callbackProtocol string, callbackPort int, oktaIssuer string) error { customerProvider := okta.New(customerClientID, customerSecret, oktaIssuer, customerCallbackUrl, "openid", "profile", "email") officeProvider := okta.New(officeClientID, officeSecret, oktaIssuer, officeCallbackUrl, "openid", "profile", "email") adminProvider := okta.New(adminClientID, adminSecret, oktaIssuer, adminCallbackUrl, "openid", "profile", "email") - + customerProvider.SetName(customerProviderName) + officeProvider.SetName(officeProviderName) + adminProvider.SetName(adminProviderName) goth.UseProviders( wrapOktaProvider(customerProvider, op.Logger), wrapOktaProvider(officeProvider, op.Logger), From 5a13d186d53c176edc7900eebdb24871410711cd Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Fri, 4 Aug 2023 13:24:23 +0000 Subject: [PATCH 04/24] Refactored okta provider code --- pkg/handlers/authentication/auth.go | 48 ++++------------------ pkg/handlers/authentication/okta.go | 64 +++++++++++++++++++---------- 2 files changed, 51 insertions(+), 61 deletions(-) diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index fc4c003dce7..d7c82c5888c 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -9,7 +9,6 @@ import ( "io" "net/http" "net/url" - "os" "strings" "time" @@ -1142,47 +1141,16 @@ func fetchToken(code string, clientID string, loginGovProvider LoginGovProvider) return &session, err } -// InitAuth initializes the Okta and Logingov provider +// InitAuth initializes the Okta provider func InitAuth(v *viper.Viper, logger *zap.Logger, appnames auth.ApplicationServername) (*OktaProvider, error) { // Create a new Okta Provider. This will be used in the creation of the additional providers for each subdomain oktaProvider := NewOktaProvider(logger) - /* - err := oktaProvider.RegisterProviders( - v.GetString(cli.OktaCustomerHostnameFlag), - v.GetString(cli.OktaTenantIssuerURLFlag), - v.GetString(cli.OktaCustomerClientIDFlag), - v.GetString(cli.OktaCustomerSecretKeyFlag), - v.GetString(cli.OktaCustomerHostnameFlag), - v.GetString(cli.OktaTenantIssuerURLFlag), - v.GetString(cli.OktaOfficeClientIDFlag), - v.GetString(cli.OktaOfficeSecretKeyFlag), - v.GetString(cli.OktaOfficeHostnameFlag), - v.GetString(cli.OktaTenantIssuerURLFlag), - v.GetString(cli.OktaAdminClientIDFlag), - v.GetString(cli.OktaAdminSecretKeyFlag), - v.GetString(cli.LoginGovOfficeClientIDFlag), - v.GetInt(cli.OktaTenantCallbackPortFlag), - v.GetString(cli.OktaTenantIssuerURLFlag)) - */ - - // TODO: Repace temporary envrc values once we get chamber access - err := oktaProvider.RegisterProviders( - os.Getenv("OKTA_CUSTOMER_HOSTNAME"), - "http://milmovelocal:3000/", - os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), - os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), - os.Getenv("OKTA_OFFICE_HOSTNAME"), - "http://officelocal:3000/", - os.Getenv("OKTA_OFFICE_CLIENT_ID"), - os.Getenv("OKTA_OFFICE_SECRET_KEY"), - os.Getenv("OKTA_ADMIN_HOSTNAME"), - "http://adminlocal:3000/", - os.Getenv("OKTA_ADMIN_CLIENT_ID"), - os.Getenv("OKTA_ADMIN_SECRET_KEY"), - "https", - 443, - os.Getenv("OKTA_TENANT_ISSUER_URL"), - ) - return oktaProvider, err + err := oktaProvider.RegisterProviders() + if err != nil { + logger.Error("Initializing auth", zap.Error(err)) + return nil, err + } + + return oktaProvider, nil } diff --git a/pkg/handlers/authentication/okta.go b/pkg/handlers/authentication/okta.go index ffba49d9591..1f8859099b2 100644 --- a/pkg/handlers/authentication/okta.go +++ b/pkg/handlers/authentication/okta.go @@ -83,16 +83,8 @@ func (op *OktaProvider) AuthorizationURL(r *http.Request) (*OktaData, error) { return &OktaData{authURL.String(), state}, nil } -// TODO: Clean up when working on callback func NewOktaProvider(logger *zap.Logger) *OktaProvider { return &OktaProvider{ - Provider: *okta.New( - os.Getenv("OKTA_OAUTH2_CLIENT_ID"), - os.Getenv("OKTA_OAUTH2_CLIENT_SECRET"), - os.Getenv("OKTA_OAUTH2_ISSUER"), - "http://milmovelocal:3000/", - "openid", "profile", "email", - ), Logger: logger, } } @@ -106,19 +98,49 @@ func wrapOktaProvider(provider *okta.Provider, logger *zap.Logger) *OktaProvider } // Function to register all three providers at once. -// TODO: Split this function up -func (op *OktaProvider) RegisterProviders(customerHostname string, customerCallbackUrl string, customerClientID string, customerSecret string, officeHostname string, officeCallbackUrl string, officeClientID string, officeSecret string, adminHostname string, adminCallbackUrl string, adminClientID string, adminSecret string, callbackProtocol string, callbackPort int, oktaIssuer string) error { - customerProvider := okta.New(customerClientID, customerSecret, oktaIssuer, customerCallbackUrl, "openid", "profile", "email") - officeProvider := okta.New(officeClientID, officeSecret, oktaIssuer, officeCallbackUrl, "openid", "profile", "email") - adminProvider := okta.New(adminClientID, adminSecret, oktaIssuer, adminCallbackUrl, "openid", "profile", "email") - customerProvider.SetName(customerProviderName) - officeProvider.SetName(officeProviderName) - adminProvider.SetName(adminProviderName) - goth.UseProviders( - wrapOktaProvider(customerProvider, op.Logger), - wrapOktaProvider(officeProvider, op.Logger), - wrapOktaProvider(adminProvider, op.Logger), - ) +// TODO: Use viper instead of os environment variables +func (op *OktaProvider) RegisterProviders() error { + // Declare OIDC scopes to be used within the providers + scope := []string{"openid", "email"} + // Register customer provider + err := op.RegisterOktaProvider(customerProviderName, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), os.Getenv("OKTA_CUSTOMER_CALLBACK_URL"), os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), scope) + if err != nil { + return err + } + // Register office provider + err = op.RegisterOktaProvider(officeProviderName, os.Getenv("OKTA_OFFICE_HOSTNAME"), os.Getenv("OKTA_OFFICE_CALLBACK_URL"), os.Getenv("OKTA_OFFICE_CLIENT_ID"), os.Getenv("OKTA_OFFICE_SECRET_KEY"), scope) + if err != nil { + return err + } + // Register admin provider + err = op.RegisterOktaProvider(adminProviderName, os.Getenv("OKTA_ADMIN_HOSTNAME"), os.Getenv("OKTA_ADMIN_CALLBACK_URL"), os.Getenv("OKTA_ADMIN_CLIENT_ID"), os.Getenv("OKTA_ADMIN_SECRET_KEY"), scope) + if err != nil { + return err + } + + return nil +} +// Create a new Okta provider and register it under the Goth providers +func (op *OktaProvider) RegisterOktaProvider(name string, hostname string, callbackUrl string, clientID string, secret string, scope []string) error { + provider := okta.New(clientID, secret, hostname, callbackUrl, scope...) + provider.SetName(name) + goth.UseProviders(wrapOktaProvider(provider, op.Logger)) + + // Check that the provider exists now + err := verifyProvider(name) + if err != nil { + op.Logger.Error("Could not verify goth provider", zap.Error(err)) + return err + } + return nil +} + +// Check if the provided provider name exists +func verifyProvider(name string) error { + _, err := goth.GetProvider(name) + if err != nil { + return err + } return nil } From 6688fdd80b5bd7b3af10b4d976b89325b9b5dfe5 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Fri, 4 Aug 2023 13:27:40 +0000 Subject: [PATCH 05/24] Added better error handling under okta register providers --- pkg/handlers/authentication/okta.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/handlers/authentication/okta.go b/pkg/handlers/authentication/okta.go index 1f8859099b2..b93101e3e3f 100644 --- a/pkg/handlers/authentication/okta.go +++ b/pkg/handlers/authentication/okta.go @@ -105,16 +105,19 @@ func (op *OktaProvider) RegisterProviders() error { // Register customer provider err := op.RegisterOktaProvider(customerProviderName, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), os.Getenv("OKTA_CUSTOMER_CALLBACK_URL"), os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), scope) if err != nil { + op.Logger.Error("Could not register customer okta provider", zap.Error(err)) return err } // Register office provider err = op.RegisterOktaProvider(officeProviderName, os.Getenv("OKTA_OFFICE_HOSTNAME"), os.Getenv("OKTA_OFFICE_CALLBACK_URL"), os.Getenv("OKTA_OFFICE_CLIENT_ID"), os.Getenv("OKTA_OFFICE_SECRET_KEY"), scope) if err != nil { + op.Logger.Error("Could not register office okta provider", zap.Error(err)) return err } // Register admin provider err = op.RegisterOktaProvider(adminProviderName, os.Getenv("OKTA_ADMIN_HOSTNAME"), os.Getenv("OKTA_ADMIN_CALLBACK_URL"), os.Getenv("OKTA_ADMIN_CLIENT_ID"), os.Getenv("OKTA_ADMIN_SECRET_KEY"), scope) if err != nil { + op.Logger.Error("Could not register admin okta provider", zap.Error(err)) return err } From 3345a17d791935ed2fd3c4740fdd6fd01868ee71 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Fri, 4 Aug 2023 13:49:44 +0000 Subject: [PATCH 06/24] Comment adjusting as well as switched okta provider name from customer to my --- pkg/handlers/authentication/auth.go | 10 +++++----- pkg/handlers/authentication/okta.go | 9 ++------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index d7c82c5888c..557eb9dbeb6 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -687,7 +687,7 @@ func invalidPermissionsResponse(appCtx appcontext.AppContext, handlerConfig hand http.Redirect(w, r, landingURL.String(), http.StatusTemporaryRedirect) } -// AuthorizationCallbackHandler handles the callback from the Login.gov authorization flow +// AuthorizationCallbackHandler handles the callback from the Okta.mil authorization flow func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx := h.AppContextFromRequest(r) if appCtx.Session() == nil { @@ -710,17 +710,17 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch err { case "access_denied": // The user has either cancelled or declined to authorize the client - appCtx.Logger().Error("ACCESS_DENIED error from login.gov") + appCtx.Logger().Error("ACCESS_DENIED error from okta.mil") case "invalid_request": - appCtx.Logger().Error("INVALID_REQUEST error from login.gov") + appCtx.Logger().Error("INVALID_REQUEST error from okta.mil") landingQuery.Add("error", "INVALID_REQUEST") default: - appCtx.Logger().Error("unknown error from login.gov") + appCtx.Logger().Error("unknown error from okta.mil") landingQuery.Add("error", "UNKNOWN_ERROR") } landingURL.RawQuery = landingQuery.Encode() http.Redirect(w, r, landingURL.String(), http.StatusTemporaryRedirect) - appCtx.Logger().Info("User redirected from login.gov", zap.String("landingURL", landingURL.String())) + appCtx.Logger().Info("User redirected from okta.mil", zap.String("landingURL", landingURL.String())) return } diff --git a/pkg/handlers/authentication/okta.go b/pkg/handlers/authentication/okta.go index b93101e3e3f..5860f5e3bcf 100644 --- a/pkg/handlers/authentication/okta.go +++ b/pkg/handlers/authentication/okta.go @@ -11,11 +11,6 @@ import ( "go.uber.org/zap" ) -const customerProviderName = "customerProvider" - -// const officeProviderName = "officeProvider" //used in the login_gov.go -// const adminProviderName = "adminProvider" // used in login_gov.go - type OktaProvider struct { okta.Provider Logger *zap.Logger @@ -29,7 +24,7 @@ type OktaData struct { // This function will select the correct provider to use based on its set name. func getOktaProviderForRequest(r *http.Request, oktaProvider OktaProvider) (goth.Provider, error) { session := auth.SessionFromRequestContext(r) - providerName := customerProviderName + providerName := milProviderName if session.IsOfficeApp() { providerName = officeProviderName } else if session.IsAdminApp() { @@ -103,7 +98,7 @@ func (op *OktaProvider) RegisterProviders() error { // Declare OIDC scopes to be used within the providers scope := []string{"openid", "email"} // Register customer provider - err := op.RegisterOktaProvider(customerProviderName, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), os.Getenv("OKTA_CUSTOMER_CALLBACK_URL"), os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), scope) + err := op.RegisterOktaProvider(milProviderName, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), os.Getenv("OKTA_CUSTOMER_CALLBACK_URL"), os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), scope) if err != nil { op.Logger.Error("Could not register customer okta provider", zap.Error(err)) return err From 2c26eb14207a4fff61962139f9bbdc0574140c27 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Wed, 9 Aug 2023 19:31:29 +0000 Subject: [PATCH 07/24] Removed okta from the authentication package and moved it into its own standalone package. Configured redirecthandler and callback handler to use Okta based secrets and providers to handle authentication for each app. Modified logout handler in preparation for upcoming session changes to reflect Okta implementation. Removed login.gov from handler auth context. Added new dependencies for Okta support. Added support for grant_type auth code flow with Okta auth. Modified sessions to accomodate for new Okta information. Removed most of login.gov support from auth.go, th rest to be pruned out after finalizing sessions. --- go.mod | 13 + go.sum | 20 ++ pkg/auth/session.go | 2 + pkg/handlers/authentication/auth.go | 123 +++++---- pkg/handlers/authentication/okta.go | 144 ---------- pkg/handlers/authentication/okta/provider.go | 258 ++++++++++++++++++ .../authentication/okta_auth_code_flow.go | 133 +++++++++ pkg/handlers/routing/base_routing_suite.go | 3 +- pkg/parser/tac/parse_test.go | 2 +- 9 files changed, 501 insertions(+), 197 deletions(-) delete mode 100644 pkg/handlers/authentication/okta.go create mode 100644 pkg/handlers/authentication/okta/provider.go create mode 100644 pkg/handlers/authentication/okta_auth_code_flow.go diff --git a/go.mod b/go.mod index 602e2846f75..0e2ce3ad917 100644 --- a/go.mod +++ b/go.mod @@ -87,6 +87,18 @@ require ( pault.ag/go/pksigner v1.0.2 ) +require ( + github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.0-20210816181553-5444fa50b93d // indirect + github.com/goccy/go-json v0.9.6 // indirect + github.com/lestrrat-go/backoff/v2 v2.0.8 // indirect + github.com/lestrrat-go/blackmagic v1.0.0 // indirect + github.com/lestrrat-go/httpcc v1.0.0 // indirect + github.com/lestrrat-go/iter v1.0.1 // indirect + github.com/lestrrat-go/jwx v1.2.21 // indirect + github.com/lestrrat-go/option v1.0.0 // indirect + github.com/patrickmn/go-cache v0.0.0-20180815053127-5633e0862627 // indirect +) + require ( atomicgo.dev/cursor v0.1.1 // indirect atomicgo.dev/keyboard v0.2.9 // indirect @@ -176,6 +188,7 @@ require ( github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/oklog/ulid v1.3.1 // indirect + github.com/okta/okta-jwt-verifier-golang v1.3.1 github.com/opentracing/opentracing-go v1.2.0 // indirect github.com/pelletier/go-toml/v2 v2.0.8 // indirect github.com/peterbourgon/diskv/v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index 23c812d1b44..1bfa365ca89 100644 --- a/go.sum +++ b/go.sum @@ -129,6 +129,7 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/decred/dcrd/crypto/blake256 v1.0.0/go.mod h1:sQl2p6Y26YV+ZOcSTP6thNdn47hh8kt6rqSlvmrXFAc= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.0-20210816181553-5444fa50b93d h1:1iy2qD6JEhHKKhUOA9IWs7mjco7lnw2qx8FsRI2wirE= github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.0-20210816181553-5444fa50b93d/go.mod h1:tmAIfUFEirG/Y8jhZ9M+h36obRZAk/1fcSpXwAVlfqE= github.com/disintegration/imaging v1.6.2 h1:w1LecBlG2Lnp8B3jk5zSuNqd7b4DXhcjwek1ei82L+c= github.com/disintegration/imaging v1.6.2/go.mod h1:44/5580QXChDfwIclfc/PCwrr44amcmDAg8hxG0Ewe4= @@ -288,6 +289,8 @@ github.com/gobuffalo/validate/v3 v3.3.3 h1:o7wkIGSvZBYBd6ChQoLxkz2y1pfmhbI4jNJYh github.com/gobuffalo/validate/v3 v3.3.3/go.mod h1:YC7FsbJ/9hW/VjQdmXPvFqvRis4vrRYFxr69WiNZw6g= github.com/gocarina/gocsv v0.0.0-20221216233619-1fea7ae8d380 h1:JJq8YZiS07gFIMYZxkbbiMrXIglG3k5JPPtdvckcnfQ= github.com/gocarina/gocsv v0.0.0-20221216233619-1fea7ae8d380/go.mod h1:5YoVOkjYAQumqlV356Hj3xeYh4BdZuLE0/nRkf2NKkI= +github.com/goccy/go-json v0.9.4/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= +github.com/goccy/go-json v0.9.6 h1:5/4CtRQdtsX0sal8fdVhTaiMN01Ri8BExZZ8iRmHQ6E= github.com/goccy/go-json v0.9.6/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/gofrs/uuid v4.0.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= @@ -522,12 +525,22 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/leodido/go-urn v1.2.4 h1:XlAE/cm/ms7TE/VMVoduSpNBoyc2dOxHs5MZSwAN63Q= github.com/leodido/go-urn v1.2.4/go.mod h1:7ZrI8mTSeBSHl/UaRyKQW1qZeMgak41ANeCNaVckg+4= +github.com/lestrrat-go/backoff/v2 v2.0.8 h1:oNb5E5isby2kiro9AgdHLv5N5tint1AnDVVf2E2un5A= github.com/lestrrat-go/backoff/v2 v2.0.8/go.mod h1:rHP/q/r9aT27n24JQLa7JhSQZCKBBOiM/uP402WwN8Y= +github.com/lestrrat-go/blackmagic v1.0.0 h1:XzdxDbuQTz0RZZEmdU7cnQxUtFUzgCSPq8RCz4BxIi4= github.com/lestrrat-go/blackmagic v1.0.0/go.mod h1:TNgH//0vYSs8VXDCfkZLgIrVTTXQELZffUV0tz3MtdQ= +github.com/lestrrat-go/codegen v1.0.0/go.mod h1:JhJw6OQAuPEfVKUCLItpaVLumDGWQznd1VaXrBk9TdM= +github.com/lestrrat-go/httpcc v1.0.0 h1:FszVC6cKfDvBKcJv646+lkh4GydQg2Z29scgUfkOpYc= github.com/lestrrat-go/httpcc v1.0.0/go.mod h1:tGS/u00Vh5N6FHNkExqGGNId8e0Big+++0Gf8MBnAvE= +github.com/lestrrat-go/iter v1.0.1 h1:q8faalr2dY6o8bV45uwrxq12bRa1ezKrB6oM9FUgN4A= github.com/lestrrat-go/iter v1.0.1/go.mod h1:zIdgO1mRKhn8l9vrZJZz9TUMMFbQbLeTsbqPDrJ/OJc= +github.com/lestrrat-go/jwx v1.2.18/go.mod h1:bWTBO7IHHVMtNunM8so9MT8wD+euEY1PzGEyCnuI2qM= +github.com/lestrrat-go/jwx v1.2.21 h1:n+yG95UMm5ZFsDdvsZmui+bqat4Cj/di4ys6XbgSlE8= github.com/lestrrat-go/jwx v1.2.21/go.mod h1:9cfxnOH7G1gN75CaJP2hKGcxFEx5sPh1abRIA/ZJVh4= +github.com/lestrrat-go/option v0.0.0-20210103042652-6f1ecfceda35/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I= +github.com/lestrrat-go/option v1.0.0 h1:WqAWL8kh8VcSoD6xjSH34/1m8yxluXQbDeKNfvFeEO4= github.com/lestrrat-go/option v1.0.0/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I= +github.com/lestrrat-go/pdebug/v3 v3.0.1/go.mod h1:za+m+Ve24yCxTEhR59N7UlnJomWwCiIqbJRmKeiADU4= github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/lib/pq v1.1.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= @@ -598,8 +611,12 @@ github.com/namsral/flag v1.7.4-pre/go.mod h1:OXldTctbM6SWH1K899kPZcf65KxJiD7Msce github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/oklog/ulid v1.3.1 h1:EGfNDEx6MqHz8B3uNV6QAib1UR2Lm97sHi3ocA6ESJ4= github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= +github.com/okta/okta-jwt-verifier-golang v1.3.1 h1:V+9W5KD3nG7xN0UYtnzXtkurGcs71bLwzPFuUGNMwdE= +github.com/okta/okta-jwt-verifier-golang v1.3.1/go.mod h1:cHffA777f7Yi4K+yDzUp89sGD5v8sk04Pc3CiT1OMR8= github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs= github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc= +github.com/patrickmn/go-cache v0.0.0-20180815053127-5633e0862627 h1:pSCLCl6joCFRnjpeojzOpEYs4q7Vditq8fySFG5ap3Y= +github.com/patrickmn/go-cache v0.0.0-20180815053127-5633e0862627/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ= github.com/pdfcpu/pdfcpu v0.2.5 h1:7jBh0EOQgxxpe35XjTtEzjHJzVMHO3ZwUn8EYNEA6Ng= github.com/pdfcpu/pdfcpu v0.2.5/go.mod h1:VLoFmLCCnUkneQe2uTjK1ZgPveTUZKGgIb2OP20+W5c= github.com/pelletier/go-toml v1.7.0/go.mod h1:vwGMzjaWMwyfHwgIBhI2YUM4fB6nL6lVAvS1LBMMhTE= @@ -821,6 +838,7 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200302210943-78000ba7a073/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20201203163018-be400aefbc4c/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= +golang.org/x/crypto v0.0.0-20201217014255-9d1352758620/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= @@ -1099,12 +1117,14 @@ golang.org/x/tools v0.0.0-20200729194436-6467de6f59a7/go.mod h1:njjCfa9FT2d7l9Bc golang.org/x/tools v0.0.0-20200804011535-6c149bb5ef0d/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= golang.org/x/tools v0.0.0-20200825202427-b303f430e36d/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= golang.org/x/tools v0.0.0-20200904185747-39188db58858/go.mod h1:Cj7w3i3Rnn0Xh82ur9kSqwfTHTeVxaDqrfMjpcNT6bE= +golang.org/x/tools v0.0.0-20200918232735-d647fc253266/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU= golang.org/x/tools v0.0.0-20200929161345-d7fc70abf50f/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU= golang.org/x/tools v0.0.0-20201110124207-079ba7bd75cd/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20201201161351-ac6f37ff4c2a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20201208233053-a543418bbed2/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= +golang.org/x/tools v0.0.0-20210114065538-d78b04bdf963/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.11/go.mod h1:SgwaegtQh8clINPpECJMqnxLv9I09HLqnW3RMqW0CA4= diff --git a/pkg/auth/session.go b/pkg/auth/session.go index c2339afc9ad..7169aa6e179 100644 --- a/pkg/auth/session.go +++ b/pkg/auth/session.go @@ -232,6 +232,8 @@ type Session struct { AdminUserRole string Roles roles.Roles Permissions []string + AccessToken string + ClientID string } // SetSessionInRequestContext modifies the request's Context() to add the session data diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index 557eb9dbeb6..65c3ffc2a55 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -9,6 +9,7 @@ import ( "io" "net/http" "net/url" + "os" "strings" "time" @@ -23,6 +24,7 @@ import ( "github.com/transcom/mymove/pkg/appcontext" "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/handlers" + "github.com/transcom/mymove/pkg/handlers/authentication/okta" "github.com/transcom/mymove/pkg/logging" "github.com/transcom/mymove/pkg/models" "github.com/transcom/mymove/pkg/models/roles" @@ -456,10 +458,8 @@ func (context *Context) GetFeatureFlag(flag string) bool { } // Context is the common handler type for auth handlers -// TODO: Remove loginGov type Context struct { - loginGovProvider LoginGovProvider - oktaProvider OktaProvider + oktaProvider okta.OktaProvider callbackTemplate string featureFlags map[string]bool } @@ -471,7 +471,7 @@ type FeatureFlag struct { } // NewAuthContext creates an Context -func NewAuthContext(_ *zap.Logger, oktaProvider OktaProvider, callbackProtocol string, callbackPort int) Context { +func NewAuthContext(_ *zap.Logger, oktaProvider okta.OktaProvider, callbackProtocol string, callbackPort int) Context { context := Context{ oktaProvider: oktaProvider, callbackTemplate: fmt.Sprintf("%s://%%s:%d/", callbackProtocol, callbackPort), @@ -479,7 +479,7 @@ func NewAuthContext(_ *zap.Logger, oktaProvider OktaProvider, callbackProtocol s return context } -// LogoutHandler handles logging the user out of login.gov +// LogoutHandler handles logging the user out of okta.mil type LogoutHandler struct { Context handlers.HandlerConfig @@ -494,6 +494,7 @@ func NewLogoutHandler(ac Context, hc handlers.HandlerConfig) LogoutHandler { return logoutHandler } +// !Needs to be finalized after sessions. func (h LogoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx := h.AppContextFromRequest(r) if appCtx.Session() != nil { @@ -507,18 +508,13 @@ func (h LogoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if appCtx.Session().IDToken != "" { var logoutURL string // All users logged in via devlocal-auth will have this IDToken. We - // don't want to make a call to login.gov for a logout URL as it will + // don't want to make a call to okta.mil for a logout URL as it will // fail for devlocal-auth'ed users. if appCtx.Session().IDToken == "devlocal" { logoutURL = redirectURL } else { - provider, err := getLoginGovProviderForRequest(r, h.loginGovProvider) - if err != nil { - appCtx.Logger().Error("Failed to get provider from request", zap.Error(err)) - http.Error(w, http.StatusText(500), http.StatusInternalServerError) - return - } - logoutURL = h.loginGovProvider.LogoutURL(redirectURL, provider.ClientKey()) + //TODO: Error handling + logoutURL, _ = h.oktaProvider.LogoutURL(appCtx.Session().Hostname, redirectURL, appCtx.Session().ClientID) } if !appCtx.Session().UserID.IsNil() { err := resetUserCurrentSessionID(appCtx) @@ -534,7 +530,7 @@ func (h LogoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx.Logger().Info("user logged out") fmt.Fprint(w, logoutURL) } else { - // Can't log out of login.gov without a token, redirect and let them re-auth + // Can't log out of okta.mil without a token, redirect and let them re-auth appCtx.Logger().Info("session exists but has an empty IDToken") if appCtx.Session().UserID != uuid.Nil { @@ -555,9 +551,9 @@ func (h LogoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } -// loginStateCookieName is the name given to the cookie storing the encrypted Login.gov state nonce. -const loginStateCookieName = "lg_state" -const loginStateCookieTTLInSecs = 1800 // 30 mins to transit through login.gov. +// loginStateCookieName is the name given to the cookie storing the encrypted okta.mil state nonce. +const loginStateCookieName = "okta_state" +const loginStateCookieTTLInSecs = 1800 // 30 mins to transit through okta.mil. // RedirectHandler handles redirection type RedirectHandler struct { @@ -579,13 +575,13 @@ func shaAsString(nonce string) string { return hex.EncodeToString(s[:]) } -// StateCookieName returns the login.gov state cookie name +// StateCookieName returns the okta.mil state cookie name func StateCookieName(session *auth.Session) string { return fmt.Sprintf("%s_%s", string(session.ApplicationName), loginStateCookieName) } -// RedirectHandler constructs the Login.gov authentication URL and redirects to it -// TODO: More login.gov to Okta +// RedirectHandler constructs the okta.mil authentication URL and redirects to it +// This will be called when logging in func (h RedirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx := h.AppContextFromRequest(r) @@ -601,8 +597,8 @@ func (h RedirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - // Hash the state/Nonce value sent to login.gov and set the result as an HttpOnly cookie - // Check this when we return from login.gov + // Hash the state/Nonce value sent to okta.mil and set the result as an HttpOnly cookie + // Check this when we return from okta.mil if appCtx.Session() == nil { appCtx.Logger().Error("Session is nil, so cannot get hostname for state Cookie") http.Error(w, http.StatusText(500), http.StatusInternalServerError) @@ -626,7 +622,7 @@ func (h RedirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx.Logger().Info("User has been redirected", zap.Any("redirectURL", loginData.RedirectURL)) } -// CallbackHandler processes a callback from login.gov +// CallbackHandler processes a callback from okta.mil type CallbackHandler struct { Context handlers.HandlerConfig @@ -646,7 +642,7 @@ func NewCallbackHandler(ac Context, hc handlers.HandlerConfig, sender notificati // invalidPermissionsResponse generates an http response when invalid // permissions are encountered. It *also* saves the session // information. This is needed so we have the necessary info to create -// a redirect to logout of login.gov +// a redirect to logout of okta.mil func invalidPermissionsResponse(appCtx appcontext.AppContext, handlerConfig handlers.HandlerConfig, authContext Context, w http.ResponseWriter, r *http.Request) { sessionManager := handlerConfig.SessionManagers().SessionManagerForApplication(appCtx.Session().ApplicationName) @@ -678,7 +674,7 @@ func invalidPermissionsResponse(appCtx appcontext.AppContext, handlerConfig hand } // We need to redirect here because we got to this handler after a - // redirect from login.gov. Our client application did not make + // redirect from okta.mil. Our client application did not make // this request, so we need to redirect to the client app so that // we can present a "pretty" error page to the user appCtx.Logger().Info("Redirect invalid permissions", @@ -731,12 +727,12 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - // Check the state value sent back from login.gov with the value saved in the cookie + // Check the state value sent back from okta.mil with the value saved in the cookie returnedState := r.URL.Query().Get("state") stateCookieName := StateCookieName(appCtx.Session()) stateCookie, err := r.Cookie(stateCookieName) if err != nil { - appCtx.Logger().Error("Getting login.gov state cookie", + appCtx.Logger().Error("Getting okta.mil state cookie", zap.String("stateCookieName", stateCookieName), zap.String("sessionUserId", appCtx.Session().UserID.String()), zap.Error(err)) @@ -744,26 +740,26 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { landingQuery.Add("error", "STATE_COOKIE_MISSING") landingURL.RawQuery = landingQuery.Encode() http.Redirect(w, r, landingURL.String(), http.StatusTemporaryRedirect) - appCtx.Logger().Info("User redirected from login.gov", zap.String("landingURL", landingURL.String())) + appCtx.Logger().Info("User redirected from okta.mil", zap.String("landingURL", landingURL.String())) return } hash := stateCookie.Value // case where user has 2 tabs open with different cookies if hash != shaAsString(returnedState) { - appCtx.Logger().Error("State returned from Login.gov does not match state value stored in cookie", + appCtx.Logger().Error("State returned from okta.mil does not match state value stored in cookie", zap.String("state", returnedState), zap.String("cookie", hash), zap.String("hash", shaAsString(returnedState))) - // Delete lg_state cookie + // Delete okta_state cookie auth.DeleteCookie(w, StateCookieName(appCtx.Session())) - appCtx.Logger().Info("lg_state cookie deleted") + appCtx.Logger().Info("okta_state cookie deleted") // This operation will delete all cookies from the session err = sessionManager.Destroy(r.Context()) if err != nil { - appCtx.Logger().Error("Deleting login.gov state cookie", zap.Error(err)) + appCtx.Logger().Error("Deleting okta.mil state cookie", zap.Error(err)) http.Error(w, http.StatusText(500), http.StatusInternalServerError) return } @@ -777,26 +773,49 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - provider, err := getLoginGovProviderForRequest(r, h.loginGovProvider) - if err != nil { - appCtx.Logger().Error("Get Goth provider", zap.Error(err)) - http.Error(w, http.StatusText(500), http.StatusInternalServerError) + // Exchange code received from login for access token. This is used during the grant_type auth flow + exchange := exchangeCode(r.URL.Query().Get("code"), r, appCtx, hash) + if exchange.Error != "" { + fmt.Println(exchange.Error) + fmt.Println(exchange.ErrorDescription) return } + // Gather Okta org url + orgURL := os.Getenv("OKTA_CUSTOMER_HOSTNAME") + if appCtx.Session().IsOfficeApp() { + orgURL = os.Getenv("OKTA_OFFICE_HOSTNAME") + } else if appCtx.Session().IsAdminApp() { + orgURL = os.Getenv("OKTA_ADMIN_HOSTNAME") + } - openIDUser, idToken, err := provider.FetchUserAndIDTokenByCode(r.URL.Query().Get("code")) - if err != nil { - appCtx.Logger().Error("Login.gov user info request", zap.Error(err)) + // Verify access token + _, verificationError := verifyToken(exchange.IdToken, returnedState, appCtx.Session(), orgURL) + + if verificationError != nil { + appCtx.Logger().Error("token exchange verification", zap.Error(err)) http.Error(w, http.StatusText(500), http.StatusInternalServerError) return } - appCtx.Session().IDToken = idToken - appCtx.Session().Email = openIDUser.Email + // Assign token values to session + appCtx.Session().IDToken = exchange.IdToken + appCtx.Session().AccessToken = exchange.AccessToken + + // Retrieve user info + profileData := getProfileData(r, appCtx, orgURL) + + // ! Continuing with sessions + // TODO: convert profiledata into struct. Previous implementation used goth.User - appCtx.Logger().Info("New Login", zap.String("OID_User", openIDUser.UserID), zap.String("OID_Email", openIDUser.Email), zap.String("Host", appCtx.Session().Hostname)) + appCtx.Session().IDToken = exchange.IdToken + appCtx.Session().Email = profileData["email"] + appCtx.Session().ClientID = profileData["aud"] - result := authorizeUser(r.Context(), appCtx, openIDUser, sessionManager, h.sender) + appCtx.Logger().Info("New Login", zap.String("Okta user", profileData["preferred_username"]), zap.String("Okta email", profileData["email"]), zap.String("Host", appCtx.Session().Hostname)) + // ! Hard coded error auth result. This is because sessions are TODO + // TODO: Implement sessions and remove hard coded auth result error + result := AuthorizationResult(2) + //result := authorizeUser(r.Context(), appCtx, profileData["sub"], sessionManager, h.sender) switch result { case authorizationResultError: http.Error(w, http.StatusText(500), http.StatusInternalServerError) @@ -812,9 +831,9 @@ func authorizeUser(ctx context.Context, appCtx appcontext.AppContext, openIDUser if err == nil { // In this case, we found an existing user associated with the - // unique login.gov UUID (aka OID_User, aka openIDUser.UserID, + // unique okta.mil UUID (aka OID_User, aka openIDUser.UserID, // aka models.User.login_gov_uuid) - appCtx.Logger().Info("Known user: found by login.gov OID_User, checking authorization", zap.String("OID_User", openIDUser.UserID), zap.String("OID_Email", openIDUser.Email), zap.String("user.id", userIdentity.ID.String()), zap.String("user.login_gov_email", userIdentity.Email)) + appCtx.Logger().Info("Known user: found by okta.mil OID_User, checking authorization", zap.String("OID_User", openIDUser.UserID), zap.String("OID_Email", openIDUser.Email), zap.String("user.id", userIdentity.ID.String()), zap.String("user.login_gov_email", userIdentity.Email)) result := AuthorizeKnownUser(ctx, appCtx, userIdentity, sessionManager) appCtx.Logger().Info("Known user authorization", zap.Any("authorizedResult", result), @@ -823,11 +842,11 @@ func authorizeUser(ctx context.Context, appCtx appcontext.AppContext, openIDUser return result } else if err == models.ErrFetchNotFound { // Never heard of them // so far In this case, we can't find an existing user - // associated with the unique login.gov UUID (aka OID_User, + // associated with the unique okta.mil UUID (aka OID_User, // aka openIDUser.UserID, aka models.User.login_gov_uuid). // The authorizeUnknownUser method tries to find a user record // with a matching email address - appCtx.Logger().Info("Unknown user: not found by login.gov OID_User, associating email and checking authorization", zap.String("OID_User", openIDUser.UserID), zap.String("OID_Email", openIDUser.Email)) + appCtx.Logger().Info("Unknown user: not found by okta.mil OID_User, associating email and checking authorization", zap.String("OID_User", openIDUser.UserID), zap.String("OID_Email", openIDUser.Email)) result := authorizeUnknownUser(ctx, appCtx, openIDUser, sessionManager, notificationSender) appCtx.Logger().Info("Unknown user authorization", zap.Any("authorizedResult", result), @@ -1024,7 +1043,7 @@ func authorizeUnknownUser(ctx context.Context, appCtx appcontext.AppContext, ope if err == nil { sysAdminEmail := notifications.GetSysAdminEmail(notificationSender) appCtx.Logger().Info( - "New user account created through Login.gov", + "New user account created through Okta.mil", zap.String("newUserID", user.ID.String()), ) email, emailErr := notifications.NewUserAccountCreated(appCtx, sysAdminEmail, user.ID, user.UpdatedAt) @@ -1057,7 +1076,7 @@ func authorizeUnknownUser(ctx context.Context, appCtx appcontext.AppContext, ope appCtx.Session().ServiceMemberID = newServiceMember.ID } else { // If in Office App or Admin App with valid user - update user's LoginGovUUID - appCtx.Logger().Error("Authorization associating login.gov UUID with user", + appCtx.Logger().Error("Authorization associating okta.mil UUID with user", zap.String("OID_User", openIDUser.UserID), zap.String("OID_Email", openIDUser.Email), zap.String("user.id", user.ID.String()), @@ -1094,6 +1113,8 @@ func authorizeUnknownUser(ctx context.Context, appCtx appcontext.AppContext, ope return authorizationResultAuthorized } +// !This func is currently a leftover from login_gov. +// TODO: Remove once Okta sessions are in place func fetchToken(code string, clientID string, loginGovProvider LoginGovProvider) (*openidConnect.Session, error) { logger := loginGovProvider.logger expiry := auth.GetExpiryTimeFromMinutes(auth.SessionExpiryInMinutes) @@ -1142,10 +1163,10 @@ func fetchToken(code string, clientID string, loginGovProvider LoginGovProvider) } // InitAuth initializes the Okta provider -func InitAuth(v *viper.Viper, logger *zap.Logger, appnames auth.ApplicationServername) (*OktaProvider, error) { +func InitAuth(v *viper.Viper, logger *zap.Logger, appnames auth.ApplicationServername) (*okta.OktaProvider, error) { // Create a new Okta Provider. This will be used in the creation of the additional providers for each subdomain - oktaProvider := NewOktaProvider(logger) + oktaProvider := okta.NewOktaProvider(logger) err := oktaProvider.RegisterProviders() if err != nil { logger.Error("Initializing auth", zap.Error(err)) diff --git a/pkg/handlers/authentication/okta.go b/pkg/handlers/authentication/okta.go deleted file mode 100644 index 5860f5e3bcf..00000000000 --- a/pkg/handlers/authentication/okta.go +++ /dev/null @@ -1,144 +0,0 @@ -package authentication - -import ( - "net/http" - "net/url" - "os" - - "github.com/markbates/goth" - "github.com/markbates/goth/providers/okta" - "github.com/transcom/mymove/pkg/auth" - "go.uber.org/zap" -) - -type OktaProvider struct { - okta.Provider - Logger *zap.Logger -} - -type OktaData struct { - RedirectURL string - Nonce string -} - -// This function will select the correct provider to use based on its set name. -func getOktaProviderForRequest(r *http.Request, oktaProvider OktaProvider) (goth.Provider, error) { - session := auth.SessionFromRequestContext(r) - providerName := milProviderName - if session.IsOfficeApp() { - providerName = officeProviderName - } else if session.IsAdminApp() { - providerName = adminProviderName - } - gothProvider, err := goth.GetProvider(providerName) - if err != nil { - return nil, err - } - - return gothProvider, nil -} - -// This function will use the OktaProvider to return the correct authorization URL to use -func (op *OktaProvider) AuthorizationURL(r *http.Request) (*OktaData, error) { - - // Retrieve the correct Okta Provider to use to get the correct authorization URL. This will choose from customer, - // office, or admin domains. - provider, err := getOktaProviderForRequest(r, *op) - if err != nil { - op.Logger.Error("Get Goth provider", zap.Error(err)) - return nil, err - } - - state := generateNonce() - - sess, err := provider.BeginAuth(state) - if err != nil { - op.Logger.Error("Goth begin auth", zap.Error(err)) - return nil, err - } - - baseURL, err := sess.GetAuthURL() - if err != nil { - op.Logger.Error("Goth get auth URL", zap.Error(err)) - return nil, err - } - - authURL, err := url.Parse(baseURL) - if err != nil { - op.Logger.Error("Parse auth URL", zap.Error(err)) - return nil, err - } - - params := authURL.Query() - params.Add("nonce", state) - params.Set("scope", "openid email") - - authURL.RawQuery = params.Encode() - - return &OktaData{authURL.String(), state}, nil -} - -func NewOktaProvider(logger *zap.Logger) *OktaProvider { - return &OktaProvider{ - Logger: logger, - } -} - -// This function allows us to wrap new registered providers with the zap logger. The initial Okta provider is already wrapped -func wrapOktaProvider(provider *okta.Provider, logger *zap.Logger) *OktaProvider { - return &OktaProvider{ - Provider: *provider, - Logger: logger, - } -} - -// Function to register all three providers at once. -// TODO: Use viper instead of os environment variables -func (op *OktaProvider) RegisterProviders() error { - // Declare OIDC scopes to be used within the providers - scope := []string{"openid", "email"} - // Register customer provider - err := op.RegisterOktaProvider(milProviderName, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), os.Getenv("OKTA_CUSTOMER_CALLBACK_URL"), os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), scope) - if err != nil { - op.Logger.Error("Could not register customer okta provider", zap.Error(err)) - return err - } - // Register office provider - err = op.RegisterOktaProvider(officeProviderName, os.Getenv("OKTA_OFFICE_HOSTNAME"), os.Getenv("OKTA_OFFICE_CALLBACK_URL"), os.Getenv("OKTA_OFFICE_CLIENT_ID"), os.Getenv("OKTA_OFFICE_SECRET_KEY"), scope) - if err != nil { - op.Logger.Error("Could not register office okta provider", zap.Error(err)) - return err - } - // Register admin provider - err = op.RegisterOktaProvider(adminProviderName, os.Getenv("OKTA_ADMIN_HOSTNAME"), os.Getenv("OKTA_ADMIN_CALLBACK_URL"), os.Getenv("OKTA_ADMIN_CLIENT_ID"), os.Getenv("OKTA_ADMIN_SECRET_KEY"), scope) - if err != nil { - op.Logger.Error("Could not register admin okta provider", zap.Error(err)) - return err - } - - return nil -} - -// Create a new Okta provider and register it under the Goth providers -func (op *OktaProvider) RegisterOktaProvider(name string, hostname string, callbackUrl string, clientID string, secret string, scope []string) error { - provider := okta.New(clientID, secret, hostname, callbackUrl, scope...) - provider.SetName(name) - goth.UseProviders(wrapOktaProvider(provider, op.Logger)) - - // Check that the provider exists now - err := verifyProvider(name) - if err != nil { - op.Logger.Error("Could not verify goth provider", zap.Error(err)) - return err - } - return nil -} - -// Check if the provided provider name exists -func verifyProvider(name string) error { - _, err := goth.GetProvider(name) - if err != nil { - return err - } - return nil -} diff --git a/pkg/handlers/authentication/okta/provider.go b/pkg/handlers/authentication/okta/provider.go new file mode 100644 index 00000000000..be21f2801b3 --- /dev/null +++ b/pkg/handlers/authentication/okta/provider.go @@ -0,0 +1,258 @@ +package okta + +import ( + "encoding/base64" + "math/rand" + "net/http" + "net/url" + "os" + + "github.com/markbates/goth" + gothOkta "github.com/markbates/goth/providers/okta" + "go.uber.org/zap" + + "github.com/transcom/mymove/pkg/auth" + "github.com/transcom/mymove/pkg/random" +) + +const MilProviderName = "milProvider" +const OfficeProviderName = "officeProvider" +const AdminProviderName = "adminProvider" + +type OktaProvider struct { + gothOkta.Provider + hostname string + logger *zap.Logger +} + +type OktaData struct { + RedirectURL string + Nonce string + GothSession goth.Session +} + +// This function will select the correct provider to use based on its set name. +func getOktaProviderForRequest(r *http.Request, oktaProvider OktaProvider) (goth.Provider, error) { + session := auth.SessionFromRequestContext(r) + + // Default the provider name to the "MilProviderName" which is the customer application + // It will update based on if office or admin app + providerName := MilProviderName + + // Set the provider name based on of it is an office or admin app. Remember, the provider is slected by its name + if session.IsOfficeApp() { + providerName = OfficeProviderName + } else if session.IsAdminApp() { + providerName = AdminProviderName + } + + // Retrieve the provider based on its name + gothProvider, err := goth.GetProvider(providerName) + if err != nil { + return nil, err + } + + return gothProvider, nil +} + +func getProviderName(r *http.Request) string { + session := auth.SessionFromRequestContext(r) + + // Set the provider name based on of it is an office or admin app. Remember, the provider is slected by its name + if session.IsOfficeApp() { + return OfficeProviderName + } else if session.IsAdminApp() { + return AdminProviderName + } + return MilProviderName +} + +// ! This func will likely come back during continuation of the sessions story +// // This function will return the ClientID of the current provider +// func (op *OktaProvider) ClientID(r *http.Request) (string, error) { +// // Default the provider name to the "MilProviderName" which is the customer application +// providerName := getProviderName(r) + +// // Retrieve the provider based on its name +// gothProvider, err := goth.GetProvider(providerName) +// if err != nil { +// return "", err +// } + +// return gothProvider.ClientKey, nil +// } + +// This function will use the OktaProvider to return the correct authorization URL to use +func (op *OktaProvider) AuthorizationURL(r *http.Request) (*OktaData, error) { + + // Retrieve the correct Okta Provider to use to get the correct authorization URL. This will choose from customer, + // office, or admin domains and use their information to create the URL. + provider, err := getOktaProviderForRequest(r, *op) + if err != nil { + op.logger.Error("Get Goth provider", zap.Error(err)) + return nil, err + } + + // Generate a new state that will later be stored in a cookie for auth + state := generateNonce() + + // Generate a session rom the provider and state (nonce) + sess, err := provider.BeginAuth(state) + if err != nil { + op.logger.Error("Goth begin auth", zap.Error(err)) + return nil, err + } + + // Use the goth.Session to generate the AuthURL. It knows this from the hostname. Currently we are not using a custom auth server + // outside of "default" (Note for Okta, "default" doesn't mean the default server, it just means a server named default) + baseURL, err := sess.GetAuthURL() + if err != nil { + op.logger.Error("Goth get auth URL", zap.Error(err)) + return nil, err + } + + // Parse URL + authURL, err := url.Parse(baseURL) + if err != nil { + op.logger.Error("Parse auth URL", zap.Error(err)) + return nil, err + } + + params := authURL.Query() + // Add the nonce and scope to the URL when getting ready to redirect to the login URL + params.Add("nonce", state) + params.Set("scope", "openid profile email") + + authURL.RawQuery = params.Encode() + + return &OktaData{authURL.String(), state, sess}, nil +} + +func NewOktaProvider(logger *zap.Logger) *OktaProvider { + return &OktaProvider{ + logger: logger, + } +} + +// This function allows us to wrap new registered providers with the zap logger. The initial Okta provider is already wrapped +// This will wrap the gothOkta provider with our own version of OktaProvider (With added methods) +func wrapOktaProvider(provider *gothOkta.Provider, logger *zap.Logger) *OktaProvider { + return &OktaProvider{ + Provider: *provider, + logger: logger, + } +} + +// Function to register all three providers at once. +// TODO: Use viper instead of os environment variables +func (op *OktaProvider) RegisterProviders() error { + + // Declare OIDC scopes to be used within the providers + scope := []string{"openid", "email", "profile"} + + // Register customer provider + err := op.RegisterOktaProvider(MilProviderName, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), os.Getenv("OKTA_CUSTOMER_CALLBACK_URL"), os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), scope) + if err != nil { + op.logger.Error("Could not register customer okta provider", zap.Error(err)) + return err + } + // Register office provider + err = op.RegisterOktaProvider(OfficeProviderName, os.Getenv("OKTA_OFFICE_HOSTNAME"), os.Getenv("OKTA_OFFICE_CALLBACK_URL"), os.Getenv("OKTA_OFFICE_CLIENT_ID"), os.Getenv("OKTA_OFFICE_SECRET_KEY"), scope) + if err != nil { + op.logger.Error("Could not register office okta provider", zap.Error(err)) + return err + } + // Register admin provider + err = op.RegisterOktaProvider(AdminProviderName, os.Getenv("OKTA_ADMIN_HOSTNAME"), os.Getenv("OKTA_ADMIN_CALLBACK_URL"), os.Getenv("OKTA_ADMIN_CLIENT_ID"), os.Getenv("OKTA_ADMIN_SECRET_KEY"), scope) + if err != nil { + op.logger.Error("Could not register admin okta provider", zap.Error(err)) + return err + } + + return nil +} + +// Create a new Okta provider and register it under the Goth providers +func (op *OktaProvider) RegisterOktaProvider(name string, hostname string, callbackUrl string, clientID string, secret string, scope []string) error { + // Use goth to create a new provider + provider := gothOkta.New(clientID, secret, hostname, callbackUrl, scope...) + // Set the name manualy + provider.SetName(name) + // Wrap + wrap := wrapOktaProvider(provider, op.logger) + // Set hostname + wrap.SetHostname(hostname) + // Assign to the active goth providers + goth.UseProviders(wrap) + + // Check that the provider exists now. The previous functions do not have error handling + err := verifyProvider(name) + if err != nil { + op.logger.Error("Could not verify goth provider", zap.Error(err)) + return err + } + return nil +} + +// Check if the provided provider name exists +func verifyProvider(name string) error { + _, err := goth.GetProvider(name) + if err != nil { + return err + } + return nil +} + +func (op OktaProvider) SetHostname(hostname string) { + op.hostname = hostname +} + +func (op OktaProvider) GetHostname() string { + return op.hostname +} + +// TokenURL returns a full URL to retrieve a user token from okta.mil +func (op OktaProvider) TokenURL(r *http.Request) string { + session := auth.SessionFromRequestContext(r) + + tokenURL := session.Hostname + "/oauth2/default/v1/token" + op.logger.Info("Session", zap.String("tokenUrl", tokenURL)) + + return tokenURL +} + +// LogoutURL returns a full URL to log out of login.gov with required params +// !Ensure proper testing after sessions have been handled +// TODO: Ensure works as intended +func (op OktaProvider) LogoutURL(hostname string, redirectURL string, clientId string) (string, error) { + logoutPath, _ := url.Parse(hostname + "/oauth2/v1/logout") + // Parameters taken from https://developers.login.gov/oidc/#logout + params := url.Values{ + "client_id": {clientId}, + "post_logout_redirect_uri": {redirectURL}, + "state": {generateNonce()}, + } + + logoutPath.RawQuery = params.Encode() + strLogoutPath := logoutPath.String() + op.logger.Info("Logout path", zap.String("strLogoutPath", strLogoutPath)) + + return strLogoutPath, nil +} + +func generateNonce() string { + nonceBytes := make([]byte, 64) + //RA Summary: gosec - G404 - Insecure random number source (rand) + //RA: gosec detected use of the insecure package math/rand rather than the more secure cryptographically secure pseudo-random number generator crypto/rand. + //RA: This particular usage is mitigated by sourcing the seed from crypto/rand in order to create the new random number using math/rand. + //RA Developer Status: Mitigated + //RA Validator: jneuner@mitre.org + //RA Validator Status: Mitigated + //RA Modified Severity: CAT III + // #nosec G404 + randomInt := rand.New(random.NewCryptoSeededSource()) + for i := 0; i < 64; i++ { + nonceBytes[i] = byte(randomInt.Int63() % 256) + } + return base64.URLEncoding.EncodeToString(nonceBytes) +} diff --git a/pkg/handlers/authentication/okta_auth_code_flow.go b/pkg/handlers/authentication/okta_auth_code_flow.go new file mode 100644 index 00000000000..4fe294f3e06 --- /dev/null +++ b/pkg/handlers/authentication/okta_auth_code_flow.go @@ -0,0 +1,133 @@ +package authentication + +import ( + "bytes" + "encoding/base64" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + + verifier "github.com/okta/okta-jwt-verifier-golang" + "github.com/transcom/mymove/pkg/appcontext" + "github.com/transcom/mymove/pkg/auth" + "go.uber.org/zap" +) + +// ! See flow here: +// ! https://developer.okta.com/docs/guides/implement-grant-type/authcode/main/ + +func getProfileData(r *http.Request, appCtx appcontext.AppContext, hostname string) map[string]string { + m := make(map[string]string) + + if appCtx.Session().AccessToken == "" { + return m + } + + reqUrl := hostname + "/oauth2/default/v1/userinfo" + + req, _ := http.NewRequest("GET", reqUrl, bytes.NewReader([]byte(""))) + h := req.Header + h.Add("Authorization", "Bearer "+appCtx.Session().AccessToken) + h.Add("Accept", "application/json") + + client := &http.Client{} + resp, _ := client.Do(req) + body, _ := io.ReadAll(resp.Body) + defer resp.Body.Close() + json.Unmarshal(body, &m) + + return m +} + +// ! Refactor after chamber is modified +func verifyToken(t string, nonce string, session *auth.Session, orgURL string) (*verifier.Jwt, error) { + + // Gather Okta information + clientID := os.Getenv("OKTA_CUSTOMER_CLIENT_ID") + if session.IsOfficeApp() { + clientID = os.Getenv("OKTA_OFFICE_CLIENT_ID") + } else if session.IsAdminApp() { + clientID = os.Getenv("OKTA_ADMIN_CLIENT_ID") + } + + tv := map[string]string{} + tv["nonce"] = nonce + tv["aud"] = clientID + + issuer := orgURL + "/oauth2/default" + jv := verifier.JwtVerifier{ + Issuer: issuer, + ClaimsToValidate: tv, + } + + result, err := jv.New().VerifyIdToken(t) + if err != nil { + return nil, fmt.Errorf("%s", err) + } + + if result != nil { + return result, nil + } + + return nil, fmt.Errorf("token could not be verified: %s", "") +} + +// ! Refactor once chamber is holding new secrets +func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext, hash string) Exchange { + session := auth.SessionFromRequestContext(r) + + appType := "CUSTOMER" + if session.IsOfficeApp() { + appType = "OFFICE" + } else if session.IsAdminApp() { + appType = "ADMIN" + } + + authHeader := base64.StdEncoding.EncodeToString( + []byte(os.Getenv("OKTA_"+appType+"_CLIENT_ID") + ":" + os.Getenv("OKTA_"+appType+"_SECRET_KEY"))) + + q := r.URL.Query() + q.Add("grant_type", "authorization_code") + q.Set("code", code) + // TODO: Replace os.Getenv + q.Add("redirect_uri", os.Getenv("OKTA_"+appType+"_CALLBACK_URL")) + + // TODO: Replace os.Getenv + url := os.Getenv("OKTA_"+appType+"_HOSTNAME") + "/oauth2/default/v1/token?" + q.Encode() + + req, _ := http.NewRequest("POST", url, bytes.NewReader([]byte(""))) + h := req.Header + h.Add("Authorization", "Basic "+authHeader) + h.Add("Accept", "application/json") + h.Add("Content-Type", "application/x-www-form-urlencoded") + h.Add("Connection", "close") + h.Add("Content-Length", "0") + + client := &http.Client{} + resp, err := client.Do(req) + if err != nil { + appCtx.Logger().Error("Code exchange", zap.Error(err)) + } + fmt.Println("t") + body, err := io.ReadAll(resp.Body) + if err != nil { + appCtx.Logger().Error("Code exchange", zap.Error(err)) + } + defer resp.Body.Close() + var exchange Exchange + json.Unmarshal(body, &exchange) + + return exchange +} + +type Exchange struct { + Error string `json:"error,omitempty"` + ErrorDescription string `json:"error_description,omitempty"` + AccessToken string `json:"access_token,omitempty"` + TokenType string `json:"token_type,omitempty"` + ExpiresIn int `json:"expires_in,omitempty"` + Scope string `json:"scope,omitempty"` + IdToken string `json:"id_token,omitempty"` +} diff --git a/pkg/handlers/routing/base_routing_suite.go b/pkg/handlers/routing/base_routing_suite.go index 1a103de564b..ae3cdd9c315 100644 --- a/pkg/handlers/routing/base_routing_suite.go +++ b/pkg/handlers/routing/base_routing_suite.go @@ -15,6 +15,7 @@ import ( "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/handlers" "github.com/transcom/mymove/pkg/handlers/authentication" + "github.com/transcom/mymove/pkg/handlers/authentication/okta" "github.com/transcom/mymove/pkg/models" "github.com/transcom/mymove/pkg/notifications" storageTest "github.com/transcom/mymove/pkg/storage/test" @@ -60,7 +61,7 @@ func (suite *BaseRoutingSuite) RoutingConfig() *Config { fakeS3 := storageTest.NewFakeS3Storage(true) handlerConfig.SetFileStorer(fakeS3) - fakeOktaProvider := authentication.NewOktaProvider(suite.Logger()) + fakeOktaProvider := okta.NewOktaProvider(suite.Logger()) authContext := authentication.NewAuthContext(suite.Logger(), *fakeOktaProvider, "http", 80) fakeFs := afero.NewMemMapFs() diff --git a/pkg/parser/tac/parse_test.go b/pkg/parser/tac/parse_test.go index e9caafca76b..5f079564fe0 100644 --- a/pkg/parser/tac/parse_test.go +++ b/pkg/parser/tac/parse_test.go @@ -6,9 +6,9 @@ import ( "testing" "time" + "github.com/stretchr/testify/suite" "go.uber.org/zap" - "github.com/stretchr/testify/suite" "github.com/transcom/mymove/pkg/models" "github.com/transcom/mymove/pkg/parser/tac" "github.com/transcom/mymove/pkg/testingsuite" From 3b87feb32548d84e94f11c80dc9738aaed5aabd2 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Thu, 10 Aug 2023 12:29:54 +0000 Subject: [PATCH 08/24] Modified auth tests to satisfy lint requirements. Includes replacing login.gov provider with okta. These tests are not finalized. --- pkg/handlers/authentication/auth_test.go | 7 ++++--- pkg/handlers/authentication/devlocal_test.go | 20 ++++++++++---------- pkg/parser/loa/parse_test.go | 2 +- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/pkg/handlers/authentication/auth_test.go b/pkg/handlers/authentication/auth_test.go index 3f08fed2a8b..b7649252756 100644 --- a/pkg/handlers/authentication/auth_test.go +++ b/pkg/handlers/authentication/auth_test.go @@ -23,6 +23,7 @@ import ( "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/factory" "github.com/transcom/mymove/pkg/handlers" + "github.com/transcom/mymove/pkg/handlers/authentication/okta" "github.com/transcom/mymove/pkg/handlers/ghcapi" "github.com/transcom/mymove/pkg/handlers/internalapi" "github.com/transcom/mymove/pkg/models" @@ -52,7 +53,7 @@ func (suite *AuthSuite) SetupTest() { // AuthContext returns a testing auth context func (suite *AuthSuite) AuthContext() Context { - return NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), + return NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", suite.callbackPort) } @@ -72,8 +73,8 @@ func TestAuthSuite(t *testing.T) { hs.PopTestSuite.TearDown() } -func fakeLoginGovProvider(logger *zap.Logger) LoginGovProvider { - return NewLoginGovProvider("fakeHostname", "secret_key", logger) +func fakeOktaProvider(logger *zap.Logger) *okta.OktaProvider { + return okta.NewOktaProvider(logger) } func (suite *AuthSuite) SetupSessionContext(ctx context.Context, session *auth.Session, sessionManager auth.SessionManager) context.Context { diff --git a/pkg/handlers/authentication/devlocal_test.go b/pkg/handlers/authentication/devlocal_test.go index 123c462b702..8c38e07490f 100644 --- a/pkg/handlers/authentication/devlocal_test.go +++ b/pkg/handlers/authentication/devlocal_test.go @@ -42,7 +42,7 @@ func (suite *AuthSuite) TestCreateUserHandlerMilMove() { req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") suite.NoError(req.ParseForm()) - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) handler := NewCreateUserHandler(authContext, handlerConfig) rr := httptest.NewRecorder() @@ -87,7 +87,7 @@ func (suite *AuthSuite) TestCreateUserHandlerOffice() { appnames := handlerConfig.AppNames() callbackPort := 1234 - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) handler := NewCreateUserHandler(authContext, handlerConfig) for _, newOfficeUser := range []struct { @@ -193,7 +193,7 @@ func (suite *AuthSuite) TestCreateUserHandlerAdmin() { req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") suite.NoError(req.ParseForm()) - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) sessionManagers := handlerConfig.SessionManagers() handler := NewCreateUserHandler(authContext, handlerConfig) @@ -250,7 +250,7 @@ func (suite *AuthSuite) TestCreateAndLoginUserHandlerFromMilMoveToMilMove() { } ctx := auth.SetSessionInRequestContext(req, &session) - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) sessionManagers := handlerConfig.SessionManagers() handler := NewCreateAndLoginUserHandler(authContext, handlerConfig) rr := httptest.NewRecorder() @@ -292,7 +292,7 @@ func (suite *AuthSuite) TestCreateAndLoginUserHandlerFromMilMoveToOffice() { req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") suite.NoError(req.ParseForm()) - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) sessionManagers := handlerConfig.SessionManagers() handler := NewCreateAndLoginUserHandler(authContext, handlerConfig) @@ -326,7 +326,7 @@ func (suite *AuthSuite) TestCreateAndLoginUserHandlerFromMilMoveToAdmin() { req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") suite.NoError(req.ParseForm()) - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) sessionManagers := handlerConfig.SessionManagers() handler := NewCreateAndLoginUserHandler(authContext, handlerConfig) @@ -360,7 +360,7 @@ func (suite *AuthSuite) TestCreateAndLoginUserHandlerFromOfficeToMilMove() { req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") suite.NoError(req.ParseForm()) - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) sessionManagers := handlerConfig.SessionManagers() handler := NewCreateAndLoginUserHandler(authContext, handlerConfig) @@ -394,7 +394,7 @@ func (suite *AuthSuite) TestCreateAndLoginUserHandlerFromOfficeToAdmin() { req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") suite.NoError(req.ParseForm()) - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) sessionManagers := handlerConfig.SessionManagers() handler := NewCreateAndLoginUserHandler(authContext, handlerConfig) @@ -426,7 +426,7 @@ func (suite *AuthSuite) TestCreateAndLoginUserHandlerFromAdminToMilMove() { req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") suite.NoError(req.ParseForm()) - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) sessionManagers := handlerConfig.SessionManagers() handler := NewCreateAndLoginUserHandler(authContext, handlerConfig) @@ -460,7 +460,7 @@ func (suite *AuthSuite) TestCreateAndLoginUserHandlerFromAdminToOffice() { req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") suite.NoError(req.ParseForm()) - authContext := NewAuthContext(suite.Logger(), fakeLoginGovProvider(suite.Logger()), "http", callbackPort) + authContext := NewAuthContext(suite.Logger(), *fakeOktaProvider(suite.Logger()), "http", callbackPort) sessionManagers := handlerConfig.SessionManagers() handler := NewCreateAndLoginUserHandler(authContext, handlerConfig) diff --git a/pkg/parser/loa/parse_test.go b/pkg/parser/loa/parse_test.go index dbe1804331c..9a1987a1038 100644 --- a/pkg/parser/loa/parse_test.go +++ b/pkg/parser/loa/parse_test.go @@ -6,9 +6,9 @@ import ( "testing" "time" + "github.com/stretchr/testify/suite" "go.uber.org/zap" - "github.com/stretchr/testify/suite" "github.com/transcom/mymove/pkg/factory" "github.com/transcom/mymove/pkg/models" "github.com/transcom/mymove/pkg/parser/loa" From 907dcfd11805386e872bfc58c865ce9f0f7881e1 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Thu, 10 Aug 2023 12:42:04 +0000 Subject: [PATCH 09/24] Added error handling and lint satisfaction --- .../authentication/okta_auth_code_flow.go | 25 +++++++++++++------ pkg/parser/loa/parse.go | 5 +++- pkg/parser/tac/parse.go | 5 +++- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/pkg/handlers/authentication/okta_auth_code_flow.go b/pkg/handlers/authentication/okta_auth_code_flow.go index 4fe294f3e06..86968f51032 100644 --- a/pkg/handlers/authentication/okta_auth_code_flow.go +++ b/pkg/handlers/authentication/okta_auth_code_flow.go @@ -10,19 +10,20 @@ import ( "os" verifier "github.com/okta/okta-jwt-verifier-golang" + "go.uber.org/zap" + "github.com/transcom/mymove/pkg/appcontext" "github.com/transcom/mymove/pkg/auth" - "go.uber.org/zap" ) // ! See flow here: // ! https://developer.okta.com/docs/guides/implement-grant-type/authcode/main/ -func getProfileData(r *http.Request, appCtx appcontext.AppContext, hostname string) map[string]string { +func getProfileData(r *http.Request, appCtx appcontext.AppContext, hostname string) (map[string]string, error) { m := make(map[string]string) if appCtx.Session().AccessToken == "" { - return m + return m, nil } reqUrl := hostname + "/oauth2/default/v1/userinfo" @@ -36,9 +37,13 @@ func getProfileData(r *http.Request, appCtx appcontext.AppContext, hostname stri resp, _ := client.Do(req) body, _ := io.ReadAll(resp.Body) defer resp.Body.Close() - json.Unmarshal(body, &m) + err := json.Unmarshal(body, &m) + if err != nil { + appCtx.Logger().Error("get profile data", zap.Error(err)) + return nil, err + } - return m + return m, nil } // ! Refactor after chamber is modified @@ -75,7 +80,7 @@ func verifyToken(t string, nonce string, session *auth.Session, orgURL string) ( } // ! Refactor once chamber is holding new secrets -func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext, hash string) Exchange { +func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext, hash string) (Exchange, error) { session := auth.SessionFromRequestContext(r) appType := "CUSTOMER" @@ -117,9 +122,13 @@ func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext, ha } defer resp.Body.Close() var exchange Exchange - json.Unmarshal(body, &exchange) + err = json.Unmarshal(body, &exchange) + if err != nil { + appCtx.Logger().Error("get profile data", zap.Error(err)) + return Exchange{}, err + } - return exchange + return exchange, nil } type Exchange struct { diff --git a/pkg/parser/loa/parse.go b/pkg/parser/loa/parse.go index a3c2f5d6e8b..3952a7778eb 100644 --- a/pkg/parser/loa/parse.go +++ b/pkg/parser/loa/parse.go @@ -41,7 +41,10 @@ func Parse(file io.Reader) ([]models.LineOfAccounting, error) { // and then proceed with parsing the rest of the file. if scanner.Scan() { columnHeaders = strings.Split(scanner.Text(), "|") - ensureFileStructMatchesColumnNames(columnHeaders) + err := ensureFileStructMatchesColumnNames(columnHeaders) + if err != nil { + return nil, errors.New("file column headers do not match") + } } // Process the lines of the .txt file into modeled codes diff --git a/pkg/parser/tac/parse.go b/pkg/parser/tac/parse.go index 9c9b42af3d0..001c9ed40ad 100644 --- a/pkg/parser/tac/parse.go +++ b/pkg/parser/tac/parse.go @@ -39,7 +39,10 @@ func Parse(file io.Reader) ([]models.TransportationAccountingCode, error) { // and then proceed with parsing the rest of the file. if scanner.Scan() { columnHeaders = strings.Split(scanner.Text(), "|") - ensureFileStructMatchesColumnNames(columnHeaders) + err := ensureFileStructMatchesColumnNames(columnHeaders) + if err != nil { + return nil, errors.New("file column headers do not match") + } } // Process the lines of the .txt file into modeled codes From 4905709e48952cb3accf4af33dc6e8effda707e3 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Fri, 4 Aug 2023 18:28:46 +0000 Subject: [PATCH 10/24] Cherry pick merge 9eaacf76cdae536d72cf8c552fe4a4f2da1e1d5a --- .github/workflows/golangci-lint.yml | 55 ++++++++++++++ pkg/models/line_of_accounting_trdm.go | 70 ------------------ pkg/models/transportation_accounting_code.go | 6 -- .../transportation_accounting_code_test.go | 73 ------------------- ...ransportation_accounting_code_trdm_file.go | 34 --------- pkg/parser/loa/parse.go | 34 +++------ pkg/parser/tac/parse.go | 26 ++----- 7 files changed, 73 insertions(+), 225 deletions(-) create mode 100644 .github/workflows/golangci-lint.yml delete mode 100644 pkg/models/line_of_accounting_trdm.go delete mode 100644 pkg/models/transportation_accounting_code_trdm_file.go diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 00000000000..f194f3a3792 --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,55 @@ +name: golangci-lint +on: + push: + branches: + - master + - main + pull_request: + +permissions: + contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + # pull-requests: read + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-go@v4 + with: + go-version: '1.20' + cache: false + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + # Require: The version of golangci-lint to use. + # When `install-mode` is `binary` (default) the value can be v1.2 or v1.2.3 or `latest` to use the latest version. + # When `install-mode` is `goinstall` the value can be v1.2.3, `latest`, or the hash of a commit. + version: v1.53 + + # Optional: working directory, useful for monorepos + # working-directory: somedir + + # Optional: golangci-lint command line arguments. + # + # Note: By default, the `.golangci.yml` file should be at the root of the repository. + # The location of the configuration file can be changed by using `--config=` + # args: --timeout=30m --config=/my/path/.golangci.yml --issues-exit-code=0 + + # Optional: show only new issues if it's a pull request. The default value is `false`. + only-new-issues: true + + # Optional: if set to true, then all caching functionality will be completely disabled, + # takes precedence over all other caching options. + # skip-cache: true + + # Optional: if set to true, then the action won't cache or restore ~/go/pkg. + # skip-pkg-cache: true + + # Optional: if set to true, then the action won't cache or restore ~/.cache/go-build. + # skip-build-cache: true + + # Optional: The mode to install golangci-lint. It can be 'binary' or 'goinstall'. + # install-mode: "goinstall" diff --git a/pkg/models/line_of_accounting_trdm.go b/pkg/models/line_of_accounting_trdm.go deleted file mode 100644 index 8eb50c553f1..00000000000 --- a/pkg/models/line_of_accounting_trdm.go +++ /dev/null @@ -1,70 +0,0 @@ -package models - -import ( - "time" -) - -// This file declares all of the column values that are received from TRDM in regard to the pipe delimited -// Line_Of_Accounting (LOA) .txt file - -// This struct only applies to the received .txt file. -// 57 values pulled from TRDM -type LineOfAccountingTrdmFileRecord struct { - LOA_SYS_ID string // Yes, this is a string not an int to stay in line with the matrix. The LineOfAccounting struct uses an int - LOA_DPT_ID string - LOA_TNSFR_DPT_NM string - LOA_BAF_ID string - LOA_TRSY_SFX_TX string - LOA_MAJ_CLM_NM string - LOA_OP_AGNCY_ID string - LOA_ALLT_SN_ID string - LOA_PGM_ELMNT_ID string - LOA_TSK_BDGT_SBLN_TX string - LOA_DF_AGNCY_ALCTN_RCPNT_ID string - LOA_JB_ORD_NM string - LOA_SBALTMT_RCPNT_ID string - LOA_WK_CNTR_RCPNT_NM string - LOA_MAJ_RMBSMT_SRC_ID string - LOA_DTL_RMBSMT_SRC_ID string - LOA_CUST_NM string - LOA_OBJ_CLS_ID string - LOA_SRV_SRC_ID string - LOA_SPCL_INTR_ID string - LOA_BDGT_ACNT_CLS_NM string - LOA_DOC_ID string - LOA_CLS_REF_ID string - LOA_INSTL_ACNTG_ACT_ID string - LOA_LCL_INSTL_ID string - LOA_FMS_TRNSACTN_ID string - LOA_DSC_TX string - LOA_BGN_DT time.Time - LOA_END_DT time.Time - LOA_FNCT_PRS_NM string - LOA_STAT_CD string - LOA_HIST_STAT_CD string - LOA_HS_GDS_CD string - ORG_GRP_DFAS_CD string - LOA_UIC string - LOA_TRNSN_ID string - LOA_SUB_ACNT_ID string - LOA_BET_CD string - LOA_FND_TY_FG_CD string - LOA_BGT_LN_ITM_ID string - LOA_SCRTY_COOP_IMPL_AGNC_CD string - LOA_SCRTY_COOP_DSGNTR_CD string - LOA_SCRTY_COOP_LN_ITM_ID string - LOA_AGNC_DSBR_CD string - LOA_AGNC_ACNTNG_CD string - LOA_FND_CNTR_ID string - LOA_CST_CNTR_ID string - LOA_PRJ_ID string - LOA_ACTVTY_ID string - LOA_CST_CD string - LOA_WRK_ORD_ID string - LOA_FNCL_AR_ID string - LOA_SCRTY_COOP_CUST_CD string - LOA_END_FY_TX int - LOA_BG_FY_TX int - LOA_BGT_RSTR_CD string - LOA_BGT_SUB_ACT_CD string -} diff --git a/pkg/models/transportation_accounting_code.go b/pkg/models/transportation_accounting_code.go index f43b605dd2e..cff9c54cd21 100644 --- a/pkg/models/transportation_accounting_code.go +++ b/pkg/models/transportation_accounting_code.go @@ -54,9 +54,3 @@ func (t *TransportationAccountingCode) Validate(_ *pop.Connection) (*validate.Er func (t TransportationAccountingCode) TableName() string { return "transportation_accounting_codes" } - -func MapTransportationAccountingCodeFileRecordToInternalStruct(tacFileRecord TransportationAccountingCodeTrdmFileRecord) TransportationAccountingCode { - return TransportationAccountingCode{ - TAC: tacFileRecord.TRNSPRTN_ACNT_CD, - } -} diff --git a/pkg/models/transportation_accounting_code_test.go b/pkg/models/transportation_accounting_code_test.go index c1097a964aa..bc1a6a57545 100644 --- a/pkg/models/transportation_accounting_code_test.go +++ b/pkg/models/transportation_accounting_code_test.go @@ -1,83 +1,10 @@ package models_test import ( - "reflect" - "testing" - "github.com/transcom/mymove/pkg/factory" "github.com/transcom/mymove/pkg/models" ) -// This test will check if any field names are not being mapped properly. This test is not finalized. -func TestTransportationAccountingCodeMapForUnusedFields(t *testing.T) { - t.Skip("Skipping TestTransportationAccountingCodeMapForUnusedFields until the fields and usecase has been finalized.") - - // Example of TransportationAccountingCodeTrdmFileRecord - tacFileRecord := models.TransportationAccountingCodeTrdmFileRecord{ - TRNSPRTN_ACNT_CD: "4EVR", - TAC_SYS_ID: "3080819", - LOA_SYS_ID: "55555555", - TAC_FY_TXT: "2023", - TAC_FN_BL_MOD_CD: "W", - ORG_GRP_DFAS_CD: "HS", - TAC_MVT_DSG_ID: "", - TAC_TY_CD: "O", - TAC_USE_CD: "N", - TAC_MAJ_CLMT_ID: "012345", - TAC_BILL_ACT_TXT: "123456", - TAC_COST_CTR_NM: "012345", - BUIC: "", - TAC_HIST_CD: "", - TAC_STAT_CD: "I", - TRNSPRTN_ACNT_TX: "For the purpose of MacDill AFB transporting to Scott AFB", - TRNSPRTN_ACNT_BGN_DT: "2022-10-01 00:00:00", - TRNSPRTN_ACNT_END_DT: "2023-09-30 00:00:00", - DD_ACTVTY_ADRS_ID: "A12345", - TAC_BLLD_ADD_FRST_LN_TX: "MacDill", - TAC_BLLD_ADD_SCND_LN_TX: "Second Address Line", - TAC_BLLD_ADD_THRD_LN_TX: "", - TAC_BLLD_ADD_FRTH_LN_TX: "TAMPA FL 33621", - TAC_FNCT_POC_NM: "THISISNOTAREALPERSON@USCG.MIL", - } - - mappedStruct := models.MapTransportationAccountingCodeFileRecordToInternalStruct(tacFileRecord) - - reflectedMappedStruct := reflect.TypeOf(mappedStruct) - reflectedTacFileRecord := reflect.TypeOf(tacFileRecord) - - // Iterate through each field in the tacRecord struct for the comparison - for i := 0; i < reflectedTacFileRecord.NumField(); i++ { - fieldName := reflectedTacFileRecord.Field(i).Name - - // Check if this field exists in the reflectedMappedStruct - _, exists := reflectedMappedStruct.FieldByName(fieldName) - - // Error if the field isn't found in the reflectedMappedStruct - if !exists { - t.Errorf("Field '%s' in TransportationAccountingCodeTrdmFileRecord is not used in MapTransportationAccountingCodeFileRecordToInternalStruct function", fieldName) - } - } -} - -// This function will test the receival of a parsed TAC that has undergone the pipe delimited .txt file parser. It will test -// that the received values correctly map to our internal TAC struct. For example, our Transporation Accounting Code is called -// "TAC" in its struct, however when it is received in pipe delimited format it will be received as "TRNSPRTN_ACNT_CD". -// This function makes sure it gets connected properly. -func TestTransportationAccountingCodeMapToInternal(t *testing.T) { - - tacFileRecord := models.TransportationAccountingCodeTrdmFileRecord{ - TRNSPRTN_ACNT_CD: "4EVR", - } - - mappedTacFileRecord := models.MapTransportationAccountingCodeFileRecordToInternalStruct(tacFileRecord) - - // Check that the TRNSPRTN_ACNT_CD field in the original struct was correctly - // mapped to the TAC field in the resulting struct - if mappedTacFileRecord.TAC != tacFileRecord.TRNSPRTN_ACNT_CD { - t.Errorf("Expected TAC to be '%s', got '%s'", tacFileRecord.TRNSPRTN_ACNT_CD, mappedTacFileRecord.TAC) - } -} - func (suite *ModelSuite) Test_CanSaveValidTac() { tac := models.TransportationAccountingCode{ TAC: "Tac1", diff --git a/pkg/models/transportation_accounting_code_trdm_file.go b/pkg/models/transportation_accounting_code_trdm_file.go deleted file mode 100644 index 4f9af20ae2d..00000000000 --- a/pkg/models/transportation_accounting_code_trdm_file.go +++ /dev/null @@ -1,34 +0,0 @@ -package models - -// This file declares all of the column values that are received from TRDM in regard to the pipe delimited -// Transportation_Accounting_Code (TAC) .txt file - -// This struct only applies to the received .txt file. -// 24 values pulled from TRDM -// Every TAC is a typ of LOA BUT not every LOA is a TAC -type TransportationAccountingCodeTrdmFileRecord struct { - TAC_SYS_ID string - LOA_SYS_ID string // Shared with LOA. This is the key - TRNSPRTN_ACNT_CD string // Could be null? If null, use LOA - TAC_FY_TXT string - TAC_FN_BL_MOD_CD string - ORG_GRP_DFAS_CD string // Shared with LOA - TAC_MVT_DSG_ID string - TAC_TY_CD string - TAC_USE_CD string - TAC_MAJ_CLMT_ID string - TAC_BILL_ACT_TXT string - TAC_COST_CTR_NM string - BUIC string - TAC_HIST_CD string - TAC_STAT_CD string - TRNSPRTN_ACNT_TX string - TRNSPRTN_ACNT_BGN_DT string - TRNSPRTN_ACNT_END_DT string - DD_ACTVTY_ADRS_ID string - TAC_BLLD_ADD_FRST_LN_TX string - TAC_BLLD_ADD_SCND_LN_TX string - TAC_BLLD_ADD_THRD_LN_TX string - TAC_BLLD_ADD_FRTH_LN_TX string - TAC_FNCT_POC_NM string -} diff --git a/pkg/parser/loa/parse.go b/pkg/parser/loa/parse.go index 3952a7778eb..5c1d57ac45e 100644 --- a/pkg/parser/loa/parse.go +++ b/pkg/parser/loa/parse.go @@ -13,6 +13,8 @@ import ( "github.com/transcom/mymove/pkg/models" ) +var expectedColumnNames = []string{"LOA_SYS_ID", "LOA_DPT_ID", "LOA_TNSFR_DPT_NM", "LOA_BAF_ID", "LOA_TRSY_SFX_TX", "LOA_MAJ_CLM_NM", "LOA_OP_AGNCY_ID", "LOA_ALLT_SN_ID", "LOA_PGM_ELMNT_ID", "LOA_TSK_BDGT_SBLN_TX", "LOA_DF_AGNCY_ALCTN_RCPNT_ID", "LOA_JB_ORD_NM", "LOA_SBALTMT_RCPNT_ID", "LOA_WK_CNTR_RCPNT_NM", "LOA_MAJ_RMBSMT_SRC_ID", "LOA_DTL_RMBSMT_SRC_ID", "LOA_CUST_NM", "LOA_OBJ_CLS_ID", "LOA_SRV_SRC_ID", "LOA_SPCL_INTR_ID", "LOA_BDGT_ACNT_CLS_NM", "LOA_DOC_ID", "LOA_CLS_REF_ID", "LOA_INSTL_ACNTG_ACT_ID", "LOA_LCL_INSTL_ID", "LOA_FMS_TRNSACTN_ID", "LOA_DSC_TX", "LOA_BGN_DT", "LOA_END_DT", "LOA_FNCT_PRS_NM", "LOA_STAT_CD", "LOA_HIST_STAT_CD", "LOA_HS_GDS_CD", "ORG_GRP_DFAS_CD", "LOA_UIC", "LOA_TRNSN_ID", "LOA_SUB_ACNT_ID", "LOA_BET_CD", "LOA_FND_TY_FG_CD", "LOA_BGT_LN_ITM_ID", "LOA_SCRTY_COOP_IMPL_AGNC_CD", "LOA_SCRTY_COOP_DSGNTR_CD", "LOA_SCRTY_COOP_LN_ITM_ID", "LOA_AGNC_DSBR_CD", "LOA_AGNC_ACNTNG_CD", "LOA_FND_CNTR_ID", "LOA_CST_CNTR_ID", "LOA_PRJ_ID", "LOA_ACTVTY_ID", "LOA_CST_CD", "LOA_WRK_ORD_ID", "LOA_FNCL_AR_ID", "LOA_SCRTY_COOP_CUST_CD", "LOA_END_FY_TX", "LOA_BG_FY_TX", "LOA_BGT_RSTR_CD", "LOA_BGT_SUB_ACT_CD"} + const timeConversionMethod = "2006-01-02 15:04:05" // Parse the pipe delimited .txt file with the following assumptions: @@ -61,27 +63,13 @@ func ensureFileStructMatchesColumnNames(columnNames []string) error { if len(columnNames) == 0 { return errors.New("column names were not parsed properly from the second line of the loa file") } - expectedColumnNames := getFieldNames(models.LineOfAccountingTrdmFileRecord{}) + if !reflect.DeepEqual(columnNames, expectedColumnNames) { return errors.New("column names parsed do not match the expected format of loa file records") } return nil } -// This function gathers the struct field names for comparison to -// line 2 of the .txt file - The columns -func getFieldNames(obj interface{}) []string { - var fieldNames []string - - t := reflect.TypeOf(obj) - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - fieldNames = append(fieldNames, field.Name) - } - - return fieldNames -} - // Removes all LOAs with an empty HHG code func PruneEmptyHhgCodes(codes []models.LineOfAccounting) []models.LineOfAccounting { var pruned []models.LineOfAccounting @@ -122,7 +110,7 @@ func processLines(scanner *bufio.Scanner, columnHeaders []string, codes []models return nil, errors.New("malformed line in the provided loa file: " + line) } - loaSysId, err := strconv.Atoi(values[0]) + loaSysID, err := strconv.Atoi(values[0]) if err != nil { return nil, errors.New("malformed line in the provided loa file: " + line) } @@ -130,16 +118,16 @@ func processLines(scanner *bufio.Scanner, columnHeaders []string, codes []models // Check if beginning date and expired date are not blank. If not blank, run time conversion, if not, leave empty. if values[27] != "" && values[28] != "" { // Parse values[27], this is the beginning date - parsedDate, err := time.Parse(timeConversionMethod, values[27]) - if err != nil { - return nil, fmt.Errorf("malformed effective date in the provided loa file: %s", err) + parsedDate, parseError := time.Parse(timeConversionMethod, values[27]) + if parseError != nil { + return nil, fmt.Errorf("malformed effective date in the provided loa file: %s", parseError) } beginningDate = parsedDate // Parse values[28], this is the ending date - parsedDate, err = time.Parse(timeConversionMethod, values[28]) - if err != nil { - return nil, fmt.Errorf("malformed effective date in the provided loa file: %s", err) + parsedDate, parseError = time.Parse(timeConversionMethod, values[28]) + if parseError != nil { + return nil, fmt.Errorf("malformed effective date in the provided loa file: %s", parseError) } endingDate = parsedDate } @@ -158,7 +146,7 @@ func processLines(scanner *bufio.Scanner, columnHeaders []string, codes []models } code := models.LineOfAccounting{ - LoaSysID: &loaSysId, + LoaSysID: &loaSysID, LoaDptID: &values[1], LoaTnsfrDptNm: &values[2], LoaBafID: &values[3], diff --git a/pkg/parser/tac/parse.go b/pkg/parser/tac/parse.go index 001c9ed40ad..b0409614d1a 100644 --- a/pkg/parser/tac/parse.go +++ b/pkg/parser/tac/parse.go @@ -13,6 +13,8 @@ import ( "github.com/transcom/mymove/pkg/models" ) +var expectedColumnNames = []string{"TAC_SYS_ID", "LOA_SYS_ID", "TRNSPRTN_ACNT_CD", "TAC_FY_TXT", "TAC_FN_BL_MOD_CD", "ORG_GRP_DFAS_CD", "TAC_MVT_DSG_ID", "TAC_TY_CD", "TAC_USE_CD", "TAC_MAJ_CLMT_ID", "TAC_BILL_ACT_TXT", "TAC_COST_CTR_NM", "BUIC", "TAC_HIST_CD", "TAC_STAT_CD", "TRNSPRTN_ACNT_TX", "TRNSPRTN_ACNT_BGN_DT", "TRNSPRTN_ACNT_END_DT", "DD_ACTVTY_ADRS_ID", "TAC_BLLD_ADD_FRST_LN_TX", "TAC_BLLD_ADD_SCND_LN_TX", "TAC_BLLD_ADD_THRD_LN_TX", "TAC_BLLD_ADD_FRTH_LN_TX", "TAC_FNCT_POC_NM"} + // Parse the pipe delimited .txt file with the following assumptions: // 1. The first and last lines are the security classification. // 2. The second line of the file are the columns that will be a 1:1 match to the TransportationAccountingCodeTrdmFileRecord struct in pipe delimited format. @@ -59,27 +61,13 @@ func ensureFileStructMatchesColumnNames(columnNames []string) error { if len(columnNames) == 0 { return errors.New("column names were not parsed properly from the second line of the tac file") } - expectedColumnNames := getFieldNames(models.TransportationAccountingCodeTrdmFileRecord{}) - if !reflect.DeepEqual(columnNames, expectedColumnNames) { + + if !reflect.DeepEqual(expectedColumnNames, expectedColumnNames) { return errors.New("column names parsed do not match the expected format of tac file records") } return nil } -// This function gathers the struct field names for comparison to -// line 2 of the .txt file - The columns -func getFieldNames(obj interface{}) []string { - var fieldNames []string - - t := reflect.TypeOf(obj) - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - fieldNames = append(fieldNames, field.Name) - } - - return fieldNames -} - // Removes all TACs with an expiration date in the past func PruneExpiredTACs(codes []models.TransportationAccountingCode) []models.TransportationAccountingCode { var pruned []models.TransportationAccountingCode @@ -149,7 +137,7 @@ func processLines(scanner *bufio.Scanner, columnHeaders []string, codes []models for scanner.Scan() { line := scanner.Text() var tacFyTxt int - var tacSysId int + var tacSysID int var loaSysID int var err error @@ -171,7 +159,7 @@ func processLines(scanner *bufio.Scanner, columnHeaders []string, codes []models // If TacSysID is not empty, convert to int if values[0] != "" { - tacSysId, err = strconv.Atoi(values[0]) + tacSysID, err = strconv.Atoi(values[0]) if err != nil { return nil, errors.New("malformed tac_sys_id in the provided tac file: " + line) } @@ -204,7 +192,7 @@ func processLines(scanner *bufio.Scanner, columnHeaders []string, codes []models } code := models.TransportationAccountingCode{ - TacSysID: &tacSysId, + TacSysID: &tacSysID, LoaSysID: &loaSysID, TAC: values[2], TacFyTxt: &tacFyTxt, From ef9ac782e5bb4aaf78169ca4f2b8e221b094c823 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Thu, 10 Aug 2023 13:04:05 +0000 Subject: [PATCH 11/24] Error handling and lint satisfaction --- pkg/handlers/authentication/auth.go | 13 +++++++++++-- pkg/handlers/authentication/okta_auth_code_flow.go | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index 65c3ffc2a55..e19a98b355b 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -774,11 +774,15 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Exchange code received from login for access token. This is used during the grant_type auth flow - exchange := exchangeCode(r.URL.Query().Get("code"), r, appCtx, hash) + exchange, err := exchangeCode(r.URL.Query().Get("code"), r, appCtx, hash) if exchange.Error != "" { fmt.Println(exchange.Error) fmt.Println(exchange.ErrorDescription) return + } else if err != nil { + appCtx.Logger().Error("exchange code for access token", zap.Error(err)) + http.Error(w, http.StatusText(500), http.StatusInternalServerError) + return } // Gather Okta org url orgURL := os.Getenv("OKTA_CUSTOMER_HOSTNAME") @@ -802,7 +806,12 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx.Session().AccessToken = exchange.AccessToken // Retrieve user info - profileData := getProfileData(r, appCtx, orgURL) + profileData, err := getProfileData(r, appCtx, orgURL) + if err != nil { + appCtx.Logger().Error("get profile data", zap.Error(err)) + http.Error(w, http.StatusText(500), http.StatusInternalServerError) + return + } // ! Continuing with sessions // TODO: convert profiledata into struct. Previous implementation used goth.User diff --git a/pkg/handlers/authentication/okta_auth_code_flow.go b/pkg/handlers/authentication/okta_auth_code_flow.go index 86968f51032..27ea5550ff8 100644 --- a/pkg/handlers/authentication/okta_auth_code_flow.go +++ b/pkg/handlers/authentication/okta_auth_code_flow.go @@ -39,7 +39,7 @@ func getProfileData(r *http.Request, appCtx appcontext.AppContext, hostname stri defer resp.Body.Close() err := json.Unmarshal(body, &m) if err != nil { - appCtx.Logger().Error("get profile data", zap.Error(err)) + appCtx.Logger().Error("could not unmarshal body", zap.Error(err)) return nil, err } From 26594a2c3be0d42a7a8fde20a66efc857b9eebc0 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Thu, 10 Aug 2023 13:49:59 +0000 Subject: [PATCH 12/24] Finalizing lint touch-ups --- pkg/cli/auth.go | 18 +++++- pkg/handlers/authentication/auth.go | 21 ++++--- pkg/handlers/authentication/auth_test.go | 2 +- pkg/handlers/authentication/okta/provider.go | 56 +++++++++---------- .../authentication/okta_auth_code_flow.go | 10 ++-- 5 files changed, 61 insertions(+), 46 deletions(-) diff --git a/pkg/cli/auth.go b/pkg/cli/auth.go index f02a54c6363..5a2d39d4e65 100644 --- a/pkg/cli/auth.go +++ b/pkg/cli/auth.go @@ -22,9 +22,6 @@ const ( //RA Validator: jneuner@mitre.org //RA Modified Severity: CAT III // #nosec G101 - // - // Additional notes for Okta flags - these variable names with "secret" are to identify the name of the flag as well - // These variables do not store the secret // ClientAuthSecretKeyFlag is the Client Auth Secret Key Flag ClientAuthSecretKeyFlag string = "client-auth-secret-key" @@ -47,16 +44,31 @@ const ( OktaTenantIssuerURLFlag string = "okta-tenant-issuer-url" OktaTenantCallbackPortFlag string = "okta-tenant-callback-port" // Okta Customer client id and secret flags + // Summary: gosec - G101 - Password Management: Hardcoded Password + // This line was flagged because of use of the word "secret" + // This line is used to identify the name of the flag. See ClientAuthSecretKeyFlag handled above. OktaCustomerSecretKeyFlag points to the flag. + // This value of this variable does not store an application secret. + // #nosec G101 OktaCustomerSecretKeyFlag string = "okta-customer-secret-key" OktaCustomerClientIDFlag string = "okta-customr-client-id" OktaCustomerHostnameFlag string = "okta-customer-hostname" OktaCustomerCallbackProtocolFlag string = "okta-customer-callback-protocol" // Okta Office client id and secret flags + // Summary: gosec - G101 - Password Management: Hardcoded Password + // This line was flagged because of use of the word "secret" + // This line is used to identify the name of the flag. See ClientAuthSecretKeyFlag handled above. OktaOfficeSecretKeyFlag points to the flag. + // This value of this variable does not store an application secret. + // #nosec G101 OktaOfficeSecretKeyFlag string = "okta-office-secret-key" OktaOfficeClientIDFlag string = "okta-office-client-id" OktaOfficeHostnameFlag string = "okta-office-hostname" OktaOfficeCallbackProtocolFlag string = "okta-office-callback-protocol" // Okta Admin client id and secret flags + // Summary: gosec - G101 - Password Management: Hardcoded Password + // This line was flagged because of use of the word "secret" + // This line is used to identify the name of the flag. See ClientAuthSecretKeyFlag handled above. OktaAdminSecretKeyFlag points to the flag. + // This value of this variable does not store an application secret. + // #nosec G101 OktaAdminSecretKeyFlag string = "okta-admin-secret-key" OktaAdminClientIDFlag string = "okta-admin-client-id" OktaAdminHostnameFlag string = "okta-admin-hostname" diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index e19a98b355b..d02b8b2829d 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -459,7 +459,7 @@ func (context *Context) GetFeatureFlag(flag string) bool { // Context is the common handler type for auth handlers type Context struct { - oktaProvider okta.OktaProvider + oktaProvider okta.Provider callbackTemplate string featureFlags map[string]bool } @@ -471,7 +471,7 @@ type FeatureFlag struct { } // NewAuthContext creates an Context -func NewAuthContext(_ *zap.Logger, oktaProvider okta.OktaProvider, callbackProtocol string, callbackPort int) Context { +func NewAuthContext(_ *zap.Logger, oktaProvider okta.Provider, callbackProtocol string, callbackPort int) Context { context := Context{ oktaProvider: oktaProvider, callbackTemplate: fmt.Sprintf("%s://%%s:%d/", callbackProtocol, callbackPort), @@ -774,7 +774,7 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Exchange code received from login for access token. This is used during the grant_type auth flow - exchange, err := exchangeCode(r.URL.Query().Get("code"), r, appCtx, hash) + exchange, err := exchangeCode(r.URL.Query().Get("code"), r, appCtx) if exchange.Error != "" { fmt.Println(exchange.Error) fmt.Println(exchange.ErrorDescription) @@ -793,7 +793,7 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Verify access token - _, verificationError := verifyToken(exchange.IdToken, returnedState, appCtx.Session(), orgURL) + _, verificationError := verifyToken(exchange.IDToken, returnedState, appCtx.Session(), orgURL) if verificationError != nil { appCtx.Logger().Error("token exchange verification", zap.Error(err)) @@ -802,11 +802,11 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Assign token values to session - appCtx.Session().IDToken = exchange.IdToken + appCtx.Session().IDToken = exchange.IDToken appCtx.Session().AccessToken = exchange.AccessToken // Retrieve user info - profileData, err := getProfileData(r, appCtx, orgURL) + profileData, err := getProfileData(appCtx, orgURL) if err != nil { appCtx.Logger().Error("get profile data", zap.Error(err)) http.Error(w, http.StatusText(500), http.StatusInternalServerError) @@ -816,7 +816,7 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // ! Continuing with sessions // TODO: convert profiledata into struct. Previous implementation used goth.User - appCtx.Session().IDToken = exchange.IdToken + appCtx.Session().IDToken = exchange.IDToken appCtx.Session().Email = profileData["email"] appCtx.Session().ClientID = profileData["aud"] @@ -824,7 +824,8 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // ! Hard coded error auth result. This is because sessions are TODO // TODO: Implement sessions and remove hard coded auth result error result := AuthorizationResult(2) - //result := authorizeUser(r.Context(), appCtx, profileData["sub"], sessionManager, h.sender) + dump := authorizeUser(r.Context(), appCtx, goth.User{}, sessionManager, h.sender) + appCtx.Logger().Info("Dumping var", zap.Any("dump", dump)) switch result { case authorizationResultError: http.Error(w, http.StatusText(500), http.StatusInternalServerError) @@ -1172,7 +1173,9 @@ func fetchToken(code string, clientID string, loginGovProvider LoginGovProvider) } // InitAuth initializes the Okta provider -func InitAuth(v *viper.Viper, logger *zap.Logger, appnames auth.ApplicationServername) (*okta.OktaProvider, error) { +func InitAuth(_ *viper.Viper, logger *zap.Logger, _ auth.ApplicationServername) (*okta.Provider, error) { + + // ! Viper and appnames can be used here to feed into all of the os.Getenv uses for future refactor // Create a new Okta Provider. This will be used in the creation of the additional providers for each subdomain oktaProvider := okta.NewOktaProvider(logger) diff --git a/pkg/handlers/authentication/auth_test.go b/pkg/handlers/authentication/auth_test.go index b7649252756..4b402397f96 100644 --- a/pkg/handlers/authentication/auth_test.go +++ b/pkg/handlers/authentication/auth_test.go @@ -73,7 +73,7 @@ func TestAuthSuite(t *testing.T) { hs.PopTestSuite.TearDown() } -func fakeOktaProvider(logger *zap.Logger) *okta.OktaProvider { +func fakeOktaProvider(logger *zap.Logger) *okta.Provider { return okta.NewOktaProvider(logger) } diff --git a/pkg/handlers/authentication/okta/provider.go b/pkg/handlers/authentication/okta/provider.go index be21f2801b3..15eee3b2382 100644 --- a/pkg/handlers/authentication/okta/provider.go +++ b/pkg/handlers/authentication/okta/provider.go @@ -19,20 +19,20 @@ const MilProviderName = "milProvider" const OfficeProviderName = "officeProvider" const AdminProviderName = "adminProvider" -type OktaProvider struct { +type Provider struct { gothOkta.Provider hostname string logger *zap.Logger } -type OktaData struct { +type Data struct { RedirectURL string Nonce string GothSession goth.Session } // This function will select the correct provider to use based on its set name. -func getOktaProviderForRequest(r *http.Request, oktaProvider OktaProvider) (goth.Provider, error) { +func getOktaProviderForRequest(r *http.Request) (goth.Provider, error) { session := auth.SessionFromRequestContext(r) // Default the provider name to the "MilProviderName" which is the customer application @@ -55,17 +55,17 @@ func getOktaProviderForRequest(r *http.Request, oktaProvider OktaProvider) (goth return gothProvider, nil } -func getProviderName(r *http.Request) string { - session := auth.SessionFromRequestContext(r) +// func getProviderName(r *http.Request) string { +// session := auth.SessionFromRequestContext(r) - // Set the provider name based on of it is an office or admin app. Remember, the provider is slected by its name - if session.IsOfficeApp() { - return OfficeProviderName - } else if session.IsAdminApp() { - return AdminProviderName - } - return MilProviderName -} +// // Set the provider name based on of it is an office or admin app. Remember, the provider is slected by its name +// if session.IsOfficeApp() { +// return OfficeProviderName +// } else if session.IsAdminApp() { +// return AdminProviderName +// } +// return MilProviderName +// } // ! This func will likely come back during continuation of the sessions story // // This function will return the ClientID of the current provider @@ -83,11 +83,11 @@ func getProviderName(r *http.Request) string { // } // This function will use the OktaProvider to return the correct authorization URL to use -func (op *OktaProvider) AuthorizationURL(r *http.Request) (*OktaData, error) { +func (op *Provider) AuthorizationURL(r *http.Request) (*Data, error) { // Retrieve the correct Okta Provider to use to get the correct authorization URL. This will choose from customer, // office, or admin domains and use their information to create the URL. - provider, err := getOktaProviderForRequest(r, *op) + provider, err := getOktaProviderForRequest(r) if err != nil { op.logger.Error("Get Goth provider", zap.Error(err)) return nil, err @@ -125,19 +125,19 @@ func (op *OktaProvider) AuthorizationURL(r *http.Request) (*OktaData, error) { authURL.RawQuery = params.Encode() - return &OktaData{authURL.String(), state, sess}, nil + return &Data{authURL.String(), state, sess}, nil } -func NewOktaProvider(logger *zap.Logger) *OktaProvider { - return &OktaProvider{ +func NewOktaProvider(logger *zap.Logger) *Provider { + return &Provider{ logger: logger, } } // This function allows us to wrap new registered providers with the zap logger. The initial Okta provider is already wrapped // This will wrap the gothOkta provider with our own version of OktaProvider (With added methods) -func wrapOktaProvider(provider *gothOkta.Provider, logger *zap.Logger) *OktaProvider { - return &OktaProvider{ +func wrapOktaProvider(provider *gothOkta.Provider, logger *zap.Logger) *Provider { + return &Provider{ Provider: *provider, logger: logger, } @@ -145,7 +145,7 @@ func wrapOktaProvider(provider *gothOkta.Provider, logger *zap.Logger) *OktaProv // Function to register all three providers at once. // TODO: Use viper instead of os environment variables -func (op *OktaProvider) RegisterProviders() error { +func (op *Provider) RegisterProviders() error { // Declare OIDC scopes to be used within the providers scope := []string{"openid", "email", "profile"} @@ -173,9 +173,9 @@ func (op *OktaProvider) RegisterProviders() error { } // Create a new Okta provider and register it under the Goth providers -func (op *OktaProvider) RegisterOktaProvider(name string, hostname string, callbackUrl string, clientID string, secret string, scope []string) error { +func (op *Provider) RegisterOktaProvider(name string, hostname string, callbackURL string, clientID string, secret string, scope []string) error { // Use goth to create a new provider - provider := gothOkta.New(clientID, secret, hostname, callbackUrl, scope...) + provider := gothOkta.New(clientID, secret, hostname, callbackURL, scope...) // Set the name manualy provider.SetName(name) // Wrap @@ -203,16 +203,16 @@ func verifyProvider(name string) error { return nil } -func (op OktaProvider) SetHostname(hostname string) { +func (op *Provider) SetHostname(hostname string) { op.hostname = hostname } -func (op OktaProvider) GetHostname() string { +func (op *Provider) GetHostname() string { return op.hostname } // TokenURL returns a full URL to retrieve a user token from okta.mil -func (op OktaProvider) TokenURL(r *http.Request) string { +func (op Provider) TokenURL(r *http.Request) string { session := auth.SessionFromRequestContext(r) tokenURL := session.Hostname + "/oauth2/default/v1/token" @@ -224,11 +224,11 @@ func (op OktaProvider) TokenURL(r *http.Request) string { // LogoutURL returns a full URL to log out of login.gov with required params // !Ensure proper testing after sessions have been handled // TODO: Ensure works as intended -func (op OktaProvider) LogoutURL(hostname string, redirectURL string, clientId string) (string, error) { +func (op Provider) LogoutURL(hostname string, redirectURL string, clientID string) (string, error) { logoutPath, _ := url.Parse(hostname + "/oauth2/v1/logout") // Parameters taken from https://developers.login.gov/oidc/#logout params := url.Values{ - "client_id": {clientId}, + "client_id": {clientID}, "post_logout_redirect_uri": {redirectURL}, "state": {generateNonce()}, } diff --git a/pkg/handlers/authentication/okta_auth_code_flow.go b/pkg/handlers/authentication/okta_auth_code_flow.go index 27ea5550ff8..130262044db 100644 --- a/pkg/handlers/authentication/okta_auth_code_flow.go +++ b/pkg/handlers/authentication/okta_auth_code_flow.go @@ -19,16 +19,16 @@ import ( // ! See flow here: // ! https://developer.okta.com/docs/guides/implement-grant-type/authcode/main/ -func getProfileData(r *http.Request, appCtx appcontext.AppContext, hostname string) (map[string]string, error) { +func getProfileData(appCtx appcontext.AppContext, hostname string) (map[string]string, error) { m := make(map[string]string) if appCtx.Session().AccessToken == "" { return m, nil } - reqUrl := hostname + "/oauth2/default/v1/userinfo" + reqURL := hostname + "/oauth2/default/v1/userinfo" - req, _ := http.NewRequest("GET", reqUrl, bytes.NewReader([]byte(""))) + req, _ := http.NewRequest("GET", reqURL, bytes.NewReader([]byte(""))) h := req.Header h.Add("Authorization", "Bearer "+appCtx.Session().AccessToken) h.Add("Accept", "application/json") @@ -80,7 +80,7 @@ func verifyToken(t string, nonce string, session *auth.Session, orgURL string) ( } // ! Refactor once chamber is holding new secrets -func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext, hash string) (Exchange, error) { +func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext) (Exchange, error) { session := auth.SessionFromRequestContext(r) appType := "CUSTOMER" @@ -138,5 +138,5 @@ type Exchange struct { TokenType string `json:"token_type,omitempty"` ExpiresIn int `json:"expires_in,omitempty"` Scope string `json:"scope,omitempty"` - IdToken string `json:"id_token,omitempty"` + IDToken string `json:"id_token,omitempty"` } From 08cedd3e776e985469ebd42aae4f17abcccf2765 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Thu, 10 Aug 2023 16:01:57 +0000 Subject: [PATCH 13/24] Removed temporary security lint due to chamber not being utilized yet. Modified error handling in auth code flow for okta. --- pkg/cli/auth.go | 95 +++++++++---------- pkg/handlers/authentication/auth.go | 2 + .../authentication/okta_auth_code_flow.go | 2 +- 3 files changed, 47 insertions(+), 52 deletions(-) diff --git a/pkg/cli/auth.go b/pkg/cli/auth.go index 5a2d39d4e65..51d74ec59c9 100644 --- a/pkg/cli/auth.go +++ b/pkg/cli/auth.go @@ -2,7 +2,6 @@ package cli import ( "fmt" - "os" "regexp" "strings" @@ -40,39 +39,33 @@ const ( // LoginGovHostnameFlag is the Login.gov Hostname Flag LoginGovHostnameFlag string = "login-gov-hostname" + // ! Verify wording after changes // Okta tenant flags - OktaTenantIssuerURLFlag string = "okta-tenant-issuer-url" - OktaTenantCallbackPortFlag string = "okta-tenant-callback-port" + /* + OktaTenantIssuerURLFlag string = "okta-tenant-issuer-url" + OktaTenantCallbackPortFlag string = "okta-tenant-callback-port" + */ // Okta Customer client id and secret flags - // Summary: gosec - G101 - Password Management: Hardcoded Password - // This line was flagged because of use of the word "secret" - // This line is used to identify the name of the flag. See ClientAuthSecretKeyFlag handled above. OktaCustomerSecretKeyFlag points to the flag. - // This value of this variable does not store an application secret. - // #nosec G101 - OktaCustomerSecretKeyFlag string = "okta-customer-secret-key" - OktaCustomerClientIDFlag string = "okta-customr-client-id" - OktaCustomerHostnameFlag string = "okta-customer-hostname" - OktaCustomerCallbackProtocolFlag string = "okta-customer-callback-protocol" + /* + OktaCustomerSecretKeyFlag string = "okta-customer-secret-key" + OktaCustomerClientIDFlag string = "okta-customr-client-id" + OktaCustomerHostnameFlag string = "okta-customer-hostname" + OktaCustomerCallbackProtocolFlag string = "okta-customer-callback-protocol" + */ // Okta Office client id and secret flags - // Summary: gosec - G101 - Password Management: Hardcoded Password - // This line was flagged because of use of the word "secret" - // This line is used to identify the name of the flag. See ClientAuthSecretKeyFlag handled above. OktaOfficeSecretKeyFlag points to the flag. - // This value of this variable does not store an application secret. - // #nosec G101 - OktaOfficeSecretKeyFlag string = "okta-office-secret-key" - OktaOfficeClientIDFlag string = "okta-office-client-id" - OktaOfficeHostnameFlag string = "okta-office-hostname" - OktaOfficeCallbackProtocolFlag string = "okta-office-callback-protocol" + /* + OktaOfficeSecretKeyFlag string = "okta-office-secret-key" + OktaOfficeClientIDFlag string = "okta-office-client-id" + OktaOfficeHostnameFlag string = "okta-office-hostname" + OktaOfficeCallbackProtocolFlag string = "okta-office-callback-protocol" + */ // Okta Admin client id and secret flags - // Summary: gosec - G101 - Password Management: Hardcoded Password - // This line was flagged because of use of the word "secret" - // This line is used to identify the name of the flag. See ClientAuthSecretKeyFlag handled above. OktaAdminSecretKeyFlag points to the flag. - // This value of this variable does not store an application secret. - // #nosec G101 - OktaAdminSecretKeyFlag string = "okta-admin-secret-key" - OktaAdminClientIDFlag string = "okta-admin-client-id" - OktaAdminHostnameFlag string = "okta-admin-hostname" - OktaAdminCallbackProtocolFlag string = "okta-admin-callback-protocol" + /* + OktaAdminSecretKeyFlag string = "okta-admin-secret-key" + OktaAdminClientIDFlag string = "okta-admin-client-id" + OktaAdminHostnameFlag string = "okta-admin-hostname" + OktaAdminCallbackProtocolFlag string = "okta-admin-callback-protocol" + */ ) type errInvalidClientID struct { @@ -97,27 +90,27 @@ func InitAuthFlags(flag *pflag.FlagSet) { // TODO: Replace Okta os.Getenv - // Okta flags - flag.String(OktaTenantIssuerURLFlag, os.Getenv("OKTA_TENANT_ISSUER_URL"), "Okta tenant issuer URL.") - flag.Int(OktaTenantCallbackPortFlag, 443, "Okta tenant callback port.") - - // Customer flags - flag.String(OktaCustomerSecretKeyFlag, os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), "Okta customer secret key.") - flag.String(OktaCustomerClientIDFlag, os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), "Okta customer client ID.") - flag.String(OktaCustomerHostnameFlag, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), "Okta customer hostname.") - flag.String(OktaCustomerCallbackProtocolFlag, os.Getenv("OKTA_CUSTOMER_CALLBACK_PROTOCOL"), "Okta customer callback protocol.") - - // Office flags - flag.String(OktaOfficeSecretKeyFlag, os.Getenv("OKTA_OFFICE_SECRET_KEY"), "Okta office secret key.") - flag.String(OktaOfficeClientIDFlag, os.Getenv("OKTA_OFFICE_CLIENT_ID"), "Okta office client ID.") - flag.String(OktaOfficeHostnameFlag, os.Getenv("OKTA_OFFICE_HOSTNAME"), "Okta office hostname.") - flag.String(OktaOfficeCallbackProtocolFlag, os.Getenv("OKTA_OFFICE_CALLBACK_PROTOCOL"), "Okta office callback protocol.") - - // Admin flags - flag.String(OktaAdminSecretKeyFlag, os.Getenv("OKTA_ADMIN_SECRET_KEY"), "Okta admin secret key.") - flag.String(OktaAdminClientIDFlag, os.Getenv("OKTA_ADMIN_CLIENT_ID"), "Okta admin client ID.") - flag.String(OktaAdminHostnameFlag, os.Getenv("OKTA_ADMIN_HOSTNAME"), "Okta admin hostname.") - flag.String(OktaAdminCallbackProtocolFlag, os.Getenv("OKTA_ADMIN_CALLBACK_PROTOCOL"), "Okta admin callback protocol.") + // // Okta flags + // flag.String(OktaTenantIssuerURLFlag, os.Getenv("OKTA_TENANT_ISSUER_URL"), "Okta tenant issuer URL.") + // flag.Int(OktaTenantCallbackPortFlag, 443, "Okta tenant callback port.") + + // // Customer flags + // flag.String(OktaCustomerSecretKeyFlag, os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), "Okta customer secret key.") + // flag.String(OktaCustomerClientIDFlag, os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), "Okta customer client ID.") + // flag.String(OktaCustomerHostnameFlag, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), "Okta customer hostname.") + // flag.String(OktaCustomerCallbackProtocolFlag, os.Getenv("OKTA_CUSTOMER_CALLBACK_PROTOCOL"), "Okta customer callback protocol.") + + // // Office flags + // flag.String(OktaOfficeSecretKeyFlag, os.Getenv("OKTA_OFFICE_SECRET_KEY"), "Okta office secret key.") + // flag.String(OktaOfficeClientIDFlag, os.Getenv("OKTA_OFFICE_CLIENT_ID"), "Okta office client ID.") + // flag.String(OktaOfficeHostnameFlag, os.Getenv("OKTA_OFFICE_HOSTNAME"), "Okta office hostname.") + // flag.String(OktaOfficeCallbackProtocolFlag, os.Getenv("OKTA_OFFICE_CALLBACK_PROTOCOL"), "Okta office callback protocol.") + + // // Admin flags + // flag.String(OktaAdminSecretKeyFlag, os.Getenv("OKTA_ADMIN_SECRET_KEY"), "Okta admin secret key.") + // flag.String(OktaAdminClientIDFlag, os.Getenv("OKTA_ADMIN_CLIENT_ID"), "Okta admin client ID.") + // flag.String(OktaAdminHostnameFlag, os.Getenv("OKTA_ADMIN_HOSTNAME"), "Okta admin hostname.") + // flag.String(OktaAdminCallbackProtocolFlag, os.Getenv("OKTA_ADMIN_CALLBACK_PROTOCOL"), "Okta admin callback protocol.") } // CheckAuth validates Auth command line flags diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index d02b8b2829d..46e659fae5c 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -823,6 +823,8 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx.Logger().Info("New Login", zap.String("Okta user", profileData["preferred_username"]), zap.String("Okta email", profileData["email"]), zap.String("Host", appCtx.Session().Hostname)) // ! Hard coded error auth result. This is because sessions are TODO // TODO: Implement sessions and remove hard coded auth result error + + // ! This will fail for now result := AuthorizationResult(2) dump := authorizeUser(r.Context(), appCtx, goth.User{}, sessionManager, h.sender) appCtx.Logger().Info("Dumping var", zap.Any("dump", dump)) diff --git a/pkg/handlers/authentication/okta_auth_code_flow.go b/pkg/handlers/authentication/okta_auth_code_flow.go index 130262044db..a82d98f4fb5 100644 --- a/pkg/handlers/authentication/okta_auth_code_flow.go +++ b/pkg/handlers/authentication/okta_auth_code_flow.go @@ -40,7 +40,7 @@ func getProfileData(appCtx appcontext.AppContext, hostname string) (map[string]s err := json.Unmarshal(body, &m) if err != nil { appCtx.Logger().Error("could not unmarshal body", zap.Error(err)) - return nil, err + // No return is intentional } return m, nil From 5a394aaead84512e69ab8b732a1a092de26cb11e Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Thu, 10 Aug 2023 20:41:11 +0000 Subject: [PATCH 14/24] re-committing without verifying with lint due to needing review errors --- .envrc | 22 +++++++ pkg/cli/auth.go | 67 +++++++++++++------- pkg/handlers/authentication/okta/provider.go | 9 ++- 3 files changed, 69 insertions(+), 29 deletions(-) diff --git a/.envrc b/.envrc index 93679883319..94fa8567ad9 100644 --- a/.envrc +++ b/.envrc @@ -130,6 +130,28 @@ export LOGIN_GOV_HOSTNAME="idp.int.identitysandbox.gov" require LOGIN_GOV_SECRET_KEY "See 'DISABLE_AWS_VAULT_WRAPPER=1 AWS_REGION=us-gov-west-1 aws-vault exec transcom-gov-dev -- chamber read app-devlocal login_gov_secret_key'" +# Okta.mil configuration + +# Tenant +export OKTA_TENANT_ORG_URL=https://test-milmove.okta.mil +export OKTA_TENANT_CALLBACK_PORT=443 +export OKTA_TENANT_CALLBACK_PROTOCOL=https + +# Customer +require OKTA_CUSTOMER_SECRET_KEY "See 'DISABLE_AWS_VAULT_WRAPPER=1 AWS_REGION=us-gov-west-1 aws-vault exec transcom-gov-dev -- chamber read app-devlocal okta-customer-secret-key'" +export OKTA_CUSTOMER_CLIENT_ID=0oa3jalqz3iCyRT9i0k6 +export OKTA_CUSTOMER_CALLBACK_URL=http://milmovelocal:3000/auth/login-gov/callback + +# Office +require OKTA_OFFICE_SECRET_KEY "See 'DISABLE_AWS_VAULT_WRAPPER=1 AWS_REGION=us-gov-west-1 aws-vault exec transcom-gov-dev -- chamber read app-devlocal okta-office-secret-key'" +export OKTA_OFFICE_CLIENT_ID=0oa3j8zgqkIUDrhOy0k6 +export OKTA_OFFICE_CALLBACK_URL=http://officelocal:3000/auth/login-gov/callback + +# Admin +require OKTA_ADMIN_SECRET_KEY "See 'DISABLE_AWS_VAULT_WRAPPER=1 AWS_REGION=us-gov-west-1 aws-vault exec transcom-gov-dev -- chamber read app-devlocal okta-admin-secret-key'" +export OKTA_ADMIN_CLIENT_ID=0oa3j9kbaTicDz0az0k6 +export OKTA_ADMIN_CALLBACK_URL=http://adminlocal:3000/auth/login-gov/callback + # JSON Web Token (JWT) config CLIENT_AUTH_SECRET_KEY=$(cat config/tls/devlocal-client_auth_secret.key) export CLIENT_AUTH_SECRET_KEY diff --git a/pkg/cli/auth.go b/pkg/cli/auth.go index 51d74ec59c9..c8e3c4c7bb7 100644 --- a/pkg/cli/auth.go +++ b/pkg/cli/auth.go @@ -39,33 +39,52 @@ const ( // LoginGovHostnameFlag is the Login.gov Hostname Flag LoginGovHostnameFlag string = "login-gov-hostname" - // ! Verify wording after changes + // Okta flags for local development environment that serves test-milmove.okta.mil // Okta tenant flags - /* - OktaTenantIssuerURLFlag string = "okta-tenant-issuer-url" - OktaTenantCallbackPortFlag string = "okta-tenant-callback-port" - */ + OktaTenantOrgURLFlag string = "okta-tenant-org-url" + // OktaTenantCallbackPortFlag is the test-milmove Callback Port Flag + OktaTenantCallbackPortFlag string = "okta-tenant-callback-port" + // OktaTenantCallbackPortFlag is the test-milmove Callback Protocol Flag + OktaTenantCallbackProtocolFlag string = "okta-tenant-callback-protocol" + // Okta Customer client id and secret flags - /* - OktaCustomerSecretKeyFlag string = "okta-customer-secret-key" - OktaCustomerClientIDFlag string = "okta-customr-client-id" - OktaCustomerHostnameFlag string = "okta-customer-hostname" - OktaCustomerCallbackProtocolFlag string = "okta-customer-callback-protocol" - */ + OktaCustomerClientIDFlag string = "okta-customer-client-id" + // RA Summary: gosec - G101 - Password Management: Hardcoded Password + // RA: This line was flagged because of use of the word "secret" + // RA: This line is used to identify the name of the flag. OktaCustomerSecretKeyFlag is the Okta Customer Application Secret Key Flag. + // RA: This value of this variable does not store an application secret. + // RA Developer Status: RA Request + // RA Validator Status: + // RA Validator: + // RA Modified Severity: + // #nosec G101 + OktaCustomerSecretKeyFlag string = "okta-customer-secret-key" + // Okta Office client id and secret flags - /* - OktaOfficeSecretKeyFlag string = "okta-office-secret-key" - OktaOfficeClientIDFlag string = "okta-office-client-id" - OktaOfficeHostnameFlag string = "okta-office-hostname" - OktaOfficeCallbackProtocolFlag string = "okta-office-callback-protocol" - */ + OktaOfficeClientIDFlag string = "okta-office-client-id" + // RA Summary: gosec - G101 - Password Management: Hardcoded Password + // RA: This line was flagged because of use of the word "secret" + // RA: This line is used to identify the name of the flag. OktaOfficeSecretKeyFlag is the Okta Office Application Secret Key Flag. + // RA: This value of this variable does not store an application secret. + // RA Developer Status: RA Request + // RA Validator Status: + // RA Validator: + // RA Modified Severity: + // #nosec G101 + OktaOfficeSecretKeyFlag string = "okta-office-secret-key" + // Okta Admin client id and secret flags - /* - OktaAdminSecretKeyFlag string = "okta-admin-secret-key" - OktaAdminClientIDFlag string = "okta-admin-client-id" - OktaAdminHostnameFlag string = "okta-admin-hostname" - OktaAdminCallbackProtocolFlag string = "okta-admin-callback-protocol" - */ + OktaAdminClientIDFlag string = "okta-admin-client-id" + // RA Summary: gosec - G101 - Password Management: Hardcoded Password + // RA: This line was flagged because of use of the word "secret" + // RA: This line is used to identify the name of the flag. OktaAdminSecretKeyFlag is the Okta Admin Application Secret Key Flag. + // RA: This value of this variable does not store an application secret. + // RA Developer Status: RA Request + // RA Validator Status: + // RA Validator: + // RA Modified Severity: + // #nosec G101 + OktaAdminSecretKeyFlag string = "okta-admin-secret-key" ) type errInvalidClientID struct { @@ -91,7 +110,7 @@ func InitAuthFlags(flag *pflag.FlagSet) { // TODO: Replace Okta os.Getenv // // Okta flags - // flag.String(OktaTenantIssuerURLFlag, os.Getenv("OKTA_TENANT_ISSUER_URL"), "Okta tenant issuer URL.") + // flag.String(OktaTenantIssuerURLFlag, os.Getenv("OKTA_TENANT_ORG_URL"), "Okta tenant issuer URL.") // flag.Int(OktaTenantCallbackPortFlag, 443, "Okta tenant callback port.") // // Customer flags diff --git a/pkg/handlers/authentication/okta/provider.go b/pkg/handlers/authentication/okta/provider.go index 15eee3b2382..8ad59479784 100644 --- a/pkg/handlers/authentication/okta/provider.go +++ b/pkg/handlers/authentication/okta/provider.go @@ -144,26 +144,25 @@ func wrapOktaProvider(provider *gothOkta.Provider, logger *zap.Logger) *Provider } // Function to register all three providers at once. -// TODO: Use viper instead of os environment variables func (op *Provider) RegisterProviders() error { // Declare OIDC scopes to be used within the providers scope := []string{"openid", "email", "profile"} - // Register customer provider - err := op.RegisterOktaProvider(MilProviderName, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), os.Getenv("OKTA_CUSTOMER_CALLBACK_URL"), os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), scope) + // Register customer provider and pull values from env variables + err := op.RegisterOktaProvider(MilProviderName, os.Getenv("OKTA_TENANT_ORG_URL"), os.Getenv("OKTA_CUSTOMER_CALLBACK_URL"), os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), scope) if err != nil { op.logger.Error("Could not register customer okta provider", zap.Error(err)) return err } // Register office provider - err = op.RegisterOktaProvider(OfficeProviderName, os.Getenv("OKTA_OFFICE_HOSTNAME"), os.Getenv("OKTA_OFFICE_CALLBACK_URL"), os.Getenv("OKTA_OFFICE_CLIENT_ID"), os.Getenv("OKTA_OFFICE_SECRET_KEY"), scope) + err = op.RegisterOktaProvider(OfficeProviderName, os.Getenv("OKTA_TENANT_ORG_URL"), os.Getenv("OKTA_OFFICE_CALLBACK_URL"), os.Getenv("OKTA_OFFICE_CLIENT_ID"), os.Getenv("OKTA_OFFICE_SECRET_KEY"), scope) if err != nil { op.logger.Error("Could not register office okta provider", zap.Error(err)) return err } // Register admin provider - err = op.RegisterOktaProvider(AdminProviderName, os.Getenv("OKTA_ADMIN_HOSTNAME"), os.Getenv("OKTA_ADMIN_CALLBACK_URL"), os.Getenv("OKTA_ADMIN_CLIENT_ID"), os.Getenv("OKTA_ADMIN_SECRET_KEY"), scope) + err = op.RegisterOktaProvider(AdminProviderName, os.Getenv("OKTA_TENANT_ORG_URL"), os.Getenv("OKTA_ADMIN_CALLBACK_URL"), os.Getenv("OKTA_ADMIN_CLIENT_ID"), os.Getenv("OKTA_ADMIN_SECRET_KEY"), scope) if err != nil { op.logger.Error("Could not register admin okta provider", zap.Error(err)) return err From 72ab970dbb9daf6755a945abef9fec0015094d1d Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Fri, 11 Aug 2023 14:51:43 +0000 Subject: [PATCH 15/24] added okta tenant url to chamber and adjusted .envrc to allow for easier updating when moving from test environment --- .envrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.envrc b/.envrc index 94fa8567ad9..144beb2eb64 100644 --- a/.envrc +++ b/.envrc @@ -133,7 +133,7 @@ require LOGIN_GOV_SECRET_KEY "See 'DISABLE_AWS_VAULT_WRAPPER=1 AWS_REGION=us-gov # Okta.mil configuration # Tenant -export OKTA_TENANT_ORG_URL=https://test-milmove.okta.mil +require OKTA_TENANT_ORG_URL "See 'DISABLE_AWS_VAULT_WRAPPER=1 AWS_REGION=us-gov-west-1 aws-vault exec transcom-gov-dev -- chamber read app-devlocal okta-tenant-org-url'" export OKTA_TENANT_CALLBACK_PORT=443 export OKTA_TENANT_CALLBACK_PROTOCOL=https From aa46a190a9d06c859f5b070e2dd1e7ef2fa482d1 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Fri, 11 Aug 2023 20:25:39 +0000 Subject: [PATCH 16/24] Refactored provider handlers based on viper. Modified cli flags --- pkg/cli/auth.go | 6 + pkg/handlers/authentication/auth.go | 28 +++-- pkg/handlers/authentication/okta/provider.go | 110 +++++++++++++++--- .../authentication/okta_auth_code_flow.go | 39 ++----- 4 files changed, 121 insertions(+), 62 deletions(-) diff --git a/pkg/cli/auth.go b/pkg/cli/auth.go index c8e3c4c7bb7..4926d79406a 100644 --- a/pkg/cli/auth.go +++ b/pkg/cli/auth.go @@ -49,6 +49,8 @@ const ( // Okta Customer client id and secret flags OktaCustomerClientIDFlag string = "okta-customer-client-id" + OktaCustomerCallbackURL string = "okta-customer-callback-url" + // RA Summary: gosec - G101 - Password Management: Hardcoded Password // RA: This line was flagged because of use of the word "secret" // RA: This line is used to identify the name of the flag. OktaCustomerSecretKeyFlag is the Okta Customer Application Secret Key Flag. @@ -62,6 +64,8 @@ const ( // Okta Office client id and secret flags OktaOfficeClientIDFlag string = "okta-office-client-id" + OktaOfficeCallbackURL string = "okta-office-callback-url" + // RA Summary: gosec - G101 - Password Management: Hardcoded Password // RA: This line was flagged because of use of the word "secret" // RA: This line is used to identify the name of the flag. OktaOfficeSecretKeyFlag is the Okta Office Application Secret Key Flag. @@ -75,6 +79,8 @@ const ( // Okta Admin client id and secret flags OktaAdminClientIDFlag string = "okta-admin-client-id" + OktaAdminCallbackURL string = "okta-admin-callback-url" + // RA Summary: gosec - G101 - Password Management: Hardcoded Password // RA: This line was flagged because of use of the word "secret" // RA: This line is used to identify the name of the flag. OktaAdminSecretKeyFlag is the Okta Admin Application Secret Key Flag. diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index 46e659fae5c..1780c3f38c3 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -9,7 +9,6 @@ import ( "io" "net/http" "net/url" - "os" "strings" "time" @@ -773,8 +772,16 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + provider, err := okta.GetOktaProviderForRequest(r) + if err != nil { + appCtx.Logger().Error("get provider", zap.Error(err)) + http.Error(w, http.StatusText(500), http.StatusInternalServerError) + return + } + // Exchange code received from login for access token. This is used during the grant_type auth flow - exchange, err := exchangeCode(r.URL.Query().Get("code"), r, appCtx) + exchange, err := exchangeCode(r.URL.Query().Get("code"), r, appCtx, *provider) + // Double error check if exchange.Error != "" { fmt.Println(exchange.Error) fmt.Println(exchange.ErrorDescription) @@ -784,16 +791,9 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { http.Error(w, http.StatusText(500), http.StatusInternalServerError) return } - // Gather Okta org url - orgURL := os.Getenv("OKTA_CUSTOMER_HOSTNAME") - if appCtx.Session().IsOfficeApp() { - orgURL = os.Getenv("OKTA_OFFICE_HOSTNAME") - } else if appCtx.Session().IsAdminApp() { - orgURL = os.Getenv("OKTA_ADMIN_HOSTNAME") - } // Verify access token - _, verificationError := verifyToken(exchange.IDToken, returnedState, appCtx.Session(), orgURL) + _, verificationError := verifyToken(exchange.IDToken, returnedState, appCtx.Session(), *provider) if verificationError != nil { appCtx.Logger().Error("token exchange verification", zap.Error(err)) @@ -806,7 +806,7 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx.Session().AccessToken = exchange.AccessToken // Retrieve user info - profileData, err := getProfileData(appCtx, orgURL) + profileData, err := getProfileData(appCtx, *provider) if err != nil { appCtx.Logger().Error("get profile data", zap.Error(err)) http.Error(w, http.StatusText(500), http.StatusInternalServerError) @@ -1175,13 +1175,11 @@ func fetchToken(code string, clientID string, loginGovProvider LoginGovProvider) } // InitAuth initializes the Okta provider -func InitAuth(_ *viper.Viper, logger *zap.Logger, _ auth.ApplicationServername) (*okta.Provider, error) { - - // ! Viper and appnames can be used here to feed into all of the os.Getenv uses for future refactor +func InitAuth(v *viper.Viper, logger *zap.Logger, _ auth.ApplicationServername) (*okta.Provider, error) { // Create a new Okta Provider. This will be used in the creation of the additional providers for each subdomain oktaProvider := okta.NewOktaProvider(logger) - err := oktaProvider.RegisterProviders() + err := oktaProvider.RegisterProviders(v) if err != nil { logger.Error("Initializing auth", zap.Error(err)) return nil, err diff --git a/pkg/handlers/authentication/okta/provider.go b/pkg/handlers/authentication/okta/provider.go index 8ad59479784..9331f323133 100644 --- a/pkg/handlers/authentication/okta/provider.go +++ b/pkg/handlers/authentication/okta/provider.go @@ -2,16 +2,18 @@ package okta import ( "encoding/base64" + "fmt" "math/rand" "net/http" "net/url" - "os" "github.com/markbates/goth" gothOkta "github.com/markbates/goth/providers/okta" + "github.com/spf13/viper" "go.uber.org/zap" "github.com/transcom/mymove/pkg/auth" + "github.com/transcom/mymove/pkg/cli" "github.com/transcom/mymove/pkg/random" ) @@ -19,10 +21,14 @@ const MilProviderName = "milProvider" const OfficeProviderName = "officeProvider" const AdminProviderName = "adminProvider" +// This provider struct will be unique to mil, office, and admin. When you run goth.getProvider() you'll now have the orgURL, clientID, and secret on hand type Provider struct { gothOkta.Provider - hostname string - logger *zap.Logger + orgURL string + callbackURL string + clientID string + secret string + logger *zap.Logger } type Data struct { @@ -32,7 +38,7 @@ type Data struct { } // This function will select the correct provider to use based on its set name. -func getOktaProviderForRequest(r *http.Request) (goth.Provider, error) { +func GetOktaProviderForRequest(r *http.Request) (*Provider, error) { session := auth.SessionFromRequestContext(r) // Default the provider name to the "MilProviderName" which is the customer application @@ -52,7 +58,23 @@ func getOktaProviderForRequest(r *http.Request) (goth.Provider, error) { return nil, err } - return gothProvider, nil + provider, err := ConvertGothProviderToOktaProvider(gothProvider) + if err != nil { + return nil, fmt.Errorf("okta provider was likely not wrapped properly when initially created and registered") + } + + return provider, nil +} + +func ConvertGothProviderToOktaProvider(gothProvider goth.Provider) (*Provider, error) { + // Conduct type assertion to retrieve the Okta.Provider values from goth.Provider + provider, ok := gothProvider.(*Provider) + if !ok { + // Received provider was not wrapped during its registration, it is of type goth.Provider but not of + // the Okta Provider type that it should be + return nil, fmt.Errorf("provided provider is not of the expected okta type") + } + return provider, nil } // func getProviderName(r *http.Request) string { @@ -87,7 +109,7 @@ func (op *Provider) AuthorizationURL(r *http.Request) (*Data, error) { // Retrieve the correct Okta Provider to use to get the correct authorization URL. This will choose from customer, // office, or admin domains and use their information to create the URL. - provider, err := getOktaProviderForRequest(r) + provider, err := GetOktaProviderForRequest(r) if err != nil { op.logger.Error("Get Goth provider", zap.Error(err)) return nil, err @@ -144,25 +166,37 @@ func wrapOktaProvider(provider *gothOkta.Provider, logger *zap.Logger) *Provider } // Function to register all three providers at once. -func (op *Provider) RegisterProviders() error { +func (op *Provider) RegisterProviders(v *viper.Viper) error { + oktaTenantOrgURL := v.GetString(cli.OktaTenantOrgURLFlag) + oktaCustomerCallbackURL := v.GetString(cli.OktaCustomerCallbackURL) + oktaCustomerClientID := v.GetString(cli.OktaCustomerClientIDFlag) + oktaCustomerSecretKey := v.GetString(cli.OktaCustomerSecretKeyFlag) + oktaOfficeCallbackURL := v.GetString(cli.OktaOfficeCallbackURL) + oktaOfficeClientID := v.GetString(cli.OktaOfficeClientIDFlag) + oktaOfficeSecretKey := v.GetString(cli.OktaOfficeSecretKeyFlag) + oktaAdminCallbackURL := v.GetString(cli.OktaAdminCallbackURL) + oktaAdminClientID := v.GetString(cli.OktaAdminClientIDFlag) + oktaAdminSecretKey := v.GetString(cli.OktaAdminSecretKeyFlag) // Declare OIDC scopes to be used within the providers scope := []string{"openid", "email", "profile"} // Register customer provider and pull values from env variables - err := op.RegisterOktaProvider(MilProviderName, os.Getenv("OKTA_TENANT_ORG_URL"), os.Getenv("OKTA_CUSTOMER_CALLBACK_URL"), os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), scope) + err := op.RegisterOktaProvider(MilProviderName, oktaTenantOrgURL, oktaCustomerCallbackURL, oktaCustomerClientID, oktaCustomerSecretKey, scope) if err != nil { op.logger.Error("Could not register customer okta provider", zap.Error(err)) return err } + // Register office provider - err = op.RegisterOktaProvider(OfficeProviderName, os.Getenv("OKTA_TENANT_ORG_URL"), os.Getenv("OKTA_OFFICE_CALLBACK_URL"), os.Getenv("OKTA_OFFICE_CLIENT_ID"), os.Getenv("OKTA_OFFICE_SECRET_KEY"), scope) + err = op.RegisterOktaProvider(OfficeProviderName, oktaTenantOrgURL, oktaOfficeCallbackURL, oktaOfficeClientID, oktaOfficeSecretKey, scope) if err != nil { op.logger.Error("Could not register office okta provider", zap.Error(err)) return err } + // Register admin provider - err = op.RegisterOktaProvider(AdminProviderName, os.Getenv("OKTA_TENANT_ORG_URL"), os.Getenv("OKTA_ADMIN_CALLBACK_URL"), os.Getenv("OKTA_ADMIN_CLIENT_ID"), os.Getenv("OKTA_ADMIN_SECRET_KEY"), scope) + err = op.RegisterOktaProvider(AdminProviderName, oktaTenantOrgURL, oktaAdminCallbackURL, oktaAdminClientID, oktaAdminSecretKey, scope) if err != nil { op.logger.Error("Could not register admin okta provider", zap.Error(err)) return err @@ -172,15 +206,18 @@ func (op *Provider) RegisterProviders() error { } // Create a new Okta provider and register it under the Goth providers -func (op *Provider) RegisterOktaProvider(name string, hostname string, callbackURL string, clientID string, secret string, scope []string) error { +func (op *Provider) RegisterOktaProvider(name string, orgURL string, callbackURL string, clientID string, secret string, scope []string) error { // Use goth to create a new provider - provider := gothOkta.New(clientID, secret, hostname, callbackURL, scope...) + provider := gothOkta.New(clientID, secret, orgURL, callbackURL, scope...) // Set the name manualy provider.SetName(name) // Wrap wrap := wrapOktaProvider(provider, op.logger) // Set hostname - wrap.SetHostname(hostname) + wrap.SetOrgURL(orgURL) + wrap.SetClientID(clientID) + wrap.SetSecret(secret) + wrap.SetCallbackURL(callbackURL) // Assign to the active goth providers goth.UseProviders(wrap) @@ -195,19 +232,56 @@ func (op *Provider) RegisterOktaProvider(name string, hostname string, callbackU // Check if the provided provider name exists func verifyProvider(name string) error { - _, err := goth.GetProvider(name) + provider, err := goth.GetProvider(name) + fmt.Println(provider) if err != nil { return err } return nil } -func (op *Provider) SetHostname(hostname string) { - op.hostname = hostname +func (op *Provider) SetSecret(secret string) { + op.secret = secret +} + +func (op *Provider) GetSecret() string { + return op.secret +} + +func (op *Provider) SetClientID(ID string) { + op.clientID = ID +} + +func (op *Provider) GetClientID() string { + return op.clientID +} + +func (op *Provider) SetOrgURL(orgURL string) { + op.orgURL = orgURL +} + +func (op *Provider) GetOrgURL() string { + return op.orgURL +} + +func (op *Provider) GetTokenURL() string { + return op.orgURL + "/oauth2/default/v1/token" +} + +func (op *Provider) GetUserInfoURL() string { + return op.orgURL + "/oauth2/default/v1/userinfo" +} + +func (op *Provider) SetCallbackURL(URL string) { + op.callbackURL = URL +} + +func (op *Provider) GetCallbackURL() string { + return op.callbackURL } -func (op *Provider) GetHostname() string { - return op.hostname +func (op *Provider) GetIssuerURL() string { + return op.orgURL + "/oauth2/default" } // TokenURL returns a full URL to retrieve a user token from okta.mil diff --git a/pkg/handlers/authentication/okta_auth_code_flow.go b/pkg/handlers/authentication/okta_auth_code_flow.go index a82d98f4fb5..7df7d4e0b19 100644 --- a/pkg/handlers/authentication/okta_auth_code_flow.go +++ b/pkg/handlers/authentication/okta_auth_code_flow.go @@ -7,26 +7,26 @@ import ( "fmt" "io" "net/http" - "os" verifier "github.com/okta/okta-jwt-verifier-golang" "go.uber.org/zap" "github.com/transcom/mymove/pkg/appcontext" "github.com/transcom/mymove/pkg/auth" + "github.com/transcom/mymove/pkg/handlers/authentication/okta" ) // ! See flow here: // ! https://developer.okta.com/docs/guides/implement-grant-type/authcode/main/ -func getProfileData(appCtx appcontext.AppContext, hostname string) (map[string]string, error) { +func getProfileData(appCtx appcontext.AppContext, provider okta.Provider) (map[string]string, error) { m := make(map[string]string) if appCtx.Session().AccessToken == "" { return m, nil } - reqURL := hostname + "/oauth2/default/v1/userinfo" + reqURL := provider.GetUserInfoURL() req, _ := http.NewRequest("GET", reqURL, bytes.NewReader([]byte(""))) h := req.Header @@ -47,21 +47,12 @@ func getProfileData(appCtx appcontext.AppContext, hostname string) (map[string]s } // ! Refactor after chamber is modified -func verifyToken(t string, nonce string, session *auth.Session, orgURL string) (*verifier.Jwt, error) { - - // Gather Okta information - clientID := os.Getenv("OKTA_CUSTOMER_CLIENT_ID") - if session.IsOfficeApp() { - clientID = os.Getenv("OKTA_OFFICE_CLIENT_ID") - } else if session.IsAdminApp() { - clientID = os.Getenv("OKTA_ADMIN_CLIENT_ID") - } - +func verifyToken(t string, nonce string, session *auth.Session, provider okta.Provider) (*verifier.Jwt, error) { tv := map[string]string{} tv["nonce"] = nonce - tv["aud"] = clientID + tv["aud"] = provider.GetClientID() - issuer := orgURL + "/oauth2/default" + issuer := provider.GetIssuerURL() jv := verifier.JwtVerifier{ Issuer: issuer, ClaimsToValidate: tv, @@ -80,27 +71,17 @@ func verifyToken(t string, nonce string, session *auth.Session, orgURL string) ( } // ! Refactor once chamber is holding new secrets -func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext) (Exchange, error) { - session := auth.SessionFromRequestContext(r) - - appType := "CUSTOMER" - if session.IsOfficeApp() { - appType = "OFFICE" - } else if session.IsAdminApp() { - appType = "ADMIN" - } +func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext, provider okta.Provider) (Exchange, error) { authHeader := base64.StdEncoding.EncodeToString( - []byte(os.Getenv("OKTA_"+appType+"_CLIENT_ID") + ":" + os.Getenv("OKTA_"+appType+"_SECRET_KEY"))) + []byte(provider.GetClientID() + ":" + provider.GetSecret())) q := r.URL.Query() q.Add("grant_type", "authorization_code") q.Set("code", code) - // TODO: Replace os.Getenv - q.Add("redirect_uri", os.Getenv("OKTA_"+appType+"_CALLBACK_URL")) + q.Add("redirect_uri", provider.GetCallbackURL()) - // TODO: Replace os.Getenv - url := os.Getenv("OKTA_"+appType+"_HOSTNAME") + "/oauth2/default/v1/token?" + q.Encode() + url := provider.GetTokenURL() + "?" + q.Encode() req, _ := http.NewRequest("POST", url, bytes.NewReader([]byte(""))) h := req.Header From b5a1fc034dbc12fe0acb2053fdc6a0656737cfda Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Mon, 14 Aug 2023 14:49:26 +0000 Subject: [PATCH 17/24] Configured logout handler and TestAuthorizationLogoutHandler test. Added new method to the okta provider for logout URL --- pkg/handlers/authentication/auth.go | 15 ++++++++++++--- pkg/handlers/authentication/auth_test.go | 9 +++------ pkg/handlers/authentication/okta/provider.go | 13 ++++++++++--- .../authentication/okta_auth_code_flow.go | 4 +--- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index 1780c3f38c3..8622ab189f6 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -512,8 +512,17 @@ func (h LogoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if appCtx.Session().IDToken == "devlocal" { logoutURL = redirectURL } else { - //TODO: Error handling - logoutURL, _ = h.oktaProvider.LogoutURL(appCtx.Session().Hostname, redirectURL, appCtx.Session().ClientID) + provider, err := okta.GetOktaProviderForRequest(r) + if err != nil { + appCtx.Logger().Error("get provider", zap.Error(err)) + http.Error(w, http.StatusText(500), http.StatusInternalServerError) + return + } + + logoutURL, err = h.oktaProvider.LogoutURL(*provider, redirectURL) + if err != nil { + appCtx.Logger().Error("failed to retrieve logout url from provider") + } } if !appCtx.Session().UserID.IsNil() { err := resetUserCurrentSessionID(appCtx) @@ -793,7 +802,7 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Verify access token - _, verificationError := verifyToken(exchange.IDToken, returnedState, appCtx.Session(), *provider) + _, verificationError := verifyToken(exchange.IDToken, returnedState, *provider) if verificationError != nil { appCtx.Logger().Error("token exchange verification", zap.Error(err)) diff --git a/pkg/handlers/authentication/auth_test.go b/pkg/handlers/authentication/auth_test.go index 4b402397f96..d026455f462 100644 --- a/pkg/handlers/authentication/auth_test.go +++ b/pkg/handlers/authentication/auth_test.go @@ -14,7 +14,6 @@ import ( "github.com/go-chi/chi/v5" "github.com/gofrs/uuid" "github.com/markbates/goth" - "github.com/markbates/goth/providers/openidConnect" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "go.uber.org/zap" @@ -139,12 +138,10 @@ func (suite *AuthSuite) TestAuthorizationLogoutHandler() { sessionManagers := handlerConfig.SessionManagers() officeSession := sessionManagers.Office authContext := suite.AuthContext() - fakeProvider := openidConnect.Provider{ - ClientKey: "some_token", - } - fakeProvider.SetName("officeProvider") - goth.UseProviders(&fakeProvider) + oktaProvider := okta.NewOktaProvider(suite.Logger()) + err := oktaProvider.RegisterOktaProvider("officeProvider", "OrgURL", "CallbackURL", fakeToken, "secret", []string{"openid", "profile", "email"}) + suite.NoError(err) handler := officeSession.LoadAndSave(NewLogoutHandler(authContext, handlerConfig)) rr := httptest.NewRecorder() diff --git a/pkg/handlers/authentication/okta/provider.go b/pkg/handlers/authentication/okta/provider.go index 9331f323133..ce7ae1867ad 100644 --- a/pkg/handlers/authentication/okta/provider.go +++ b/pkg/handlers/authentication/okta/provider.go @@ -284,6 +284,10 @@ func (op *Provider) GetIssuerURL() string { return op.orgURL + "/oauth2/default" } +func (op *Provider) GetLogoutURL() string { + return op.orgURL + "/oauth2/v1/logout" +} + // TokenURL returns a full URL to retrieve a user token from okta.mil func (op Provider) TokenURL(r *http.Request) string { session := auth.SessionFromRequestContext(r) @@ -297,11 +301,14 @@ func (op Provider) TokenURL(r *http.Request) string { // LogoutURL returns a full URL to log out of login.gov with required params // !Ensure proper testing after sessions have been handled // TODO: Ensure works as intended -func (op Provider) LogoutURL(hostname string, redirectURL string, clientID string) (string, error) { - logoutPath, _ := url.Parse(hostname + "/oauth2/v1/logout") +func (op Provider) LogoutURL(provider Provider, redirectURL string) (string, error) { + logoutPath, err := url.Parse(provider.GetLogoutURL()) + if err != nil { + return "", err + } // Parameters taken from https://developers.login.gov/oidc/#logout params := url.Values{ - "client_id": {clientID}, + "client_id": {provider.clientID}, "post_logout_redirect_uri": {redirectURL}, "state": {generateNonce()}, } diff --git a/pkg/handlers/authentication/okta_auth_code_flow.go b/pkg/handlers/authentication/okta_auth_code_flow.go index 7df7d4e0b19..d961f3018c8 100644 --- a/pkg/handlers/authentication/okta_auth_code_flow.go +++ b/pkg/handlers/authentication/okta_auth_code_flow.go @@ -12,7 +12,6 @@ import ( "go.uber.org/zap" "github.com/transcom/mymove/pkg/appcontext" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/handlers/authentication/okta" ) @@ -46,8 +45,7 @@ func getProfileData(appCtx appcontext.AppContext, provider okta.Provider) (map[s return m, nil } -// ! Refactor after chamber is modified -func verifyToken(t string, nonce string, session *auth.Session, provider okta.Provider) (*verifier.Jwt, error) { +func verifyToken(t string, nonce string, provider okta.Provider) (*verifier.Jwt, error) { tv := map[string]string{} tv["nonce"] = nonce tv["aud"] = provider.GetClientID() From 8fb28e161e45bf0452893e0b99586be992cbb52c Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Tue, 15 Aug 2023 19:29:52 +0000 Subject: [PATCH 18/24] Created okta provider factory. Conducted some refactoring to existing okta provider code and handlers. Modified callback handler to receive a http client that can be leveraged for testing. Successfully created and implemented okta mock endpoints in tests. Auth handler test package successfully passing. --- go.mod | 1 + go.sum | 3 + pkg/factory/okta_factory.go | 45 ++++ pkg/handlers/authentication/auth.go | 32 ++- pkg/handlers/authentication/auth_test.go | 232 ++++++++++++------ pkg/handlers/authentication/okta/provider.go | 52 +--- .../authentication/okta/provider_test.go | 35 +++ .../authentication/okta_auth_code_flow.go | 18 +- 8 files changed, 277 insertions(+), 141 deletions(-) create mode 100644 pkg/factory/okta_factory.go create mode 100644 pkg/handlers/authentication/okta/provider_test.go diff --git a/go.mod b/go.mod index 0a0a7a757ed..4a2d758ea0f 100644 --- a/go.mod +++ b/go.mod @@ -187,6 +187,7 @@ require ( github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect github.com/jackc/pgtype v1.14.0 // indirect github.com/jackc/pgx/v4 v4.18.1 // indirect + github.com/jarcoal/httpmock v1.3.0 github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/joho/godotenv v1.5.1 // indirect github.com/josharian/intern v1.0.0 // indirect diff --git a/go.sum b/go.sum index 1aa5a0bf22d..0807564a460 100644 --- a/go.sum +++ b/go.sum @@ -527,6 +527,8 @@ github.com/jackc/puddle v0.0.0-20190608224051-11cab39313c9/go.mod h1:m4B5Dj62Y0f github.com/jackc/puddle v1.1.3/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk= github.com/jackc/puddle v1.3.0/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk= github.com/jarcoal/httpmock v0.0.0-20180424175123-9c70cfe4a1da/go.mod h1:ks+b9deReOc7jgqp+e7LuFiCBH6Rm5hL32cLcEAArb4= +github.com/jarcoal/httpmock v1.3.0 h1:2RJ8GP0IIaWwcC9Fp2BmVi8Kog3v2Hn7VXM3fTd+nuc= +github.com/jarcoal/httpmock v1.3.0/go.mod h1:3yb8rc4BI7TCBhFY8ng0gjuLKJNquuDNiPaZjnENuYg= github.com/jessevdk/go-flags v1.5.0 h1:1jKYvbxEjfUl0fmqTCOfonvskHHXMjBySTLW4y9LFvc= github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4= github.com/jinzhu/copier v0.3.5 h1:GlvfUwHk62RokgqVNvYsku0TATCF7bAHVwEXoBh3iJg= @@ -635,6 +637,7 @@ github.com/mattn/go-sqlite3 v1.14.15/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S github.com/mattn/go-sqlite3 v1.14.16/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S2DGjv9HUNg= github.com/mattn/go-sqlite3 v2.0.3+incompatible h1:gXHsfypPkaMZrKbD5209QV9jbUTJKjyR5WD3HYQSd+U= github.com/mattn/go-sqlite3 v2.0.3+incompatible/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc= +github.com/maxatome/go-testdeep v1.12.0 h1:Ql7Go8Tg0C1D/uMMX59LAoYK7LffeJQ6X2T04nTH68g= github.com/microcosm-cc/bluemonday v1.0.20/go.mod h1:yfBmMi8mxvaZut3Yytv+jTXRY8mxyjJ0/kQBTElld50= github.com/microcosm-cc/bluemonday v1.0.23 h1:SMZe2IGa0NuHvnVNAZ+6B38gsTbi5e4sViiWJyDDqFY= github.com/microcosm-cc/bluemonday v1.0.23/go.mod h1:mN70sk7UkkF8TUr2IGBpNN0jAgStuPzlK76QuruE/z4= diff --git a/pkg/factory/okta_factory.go b/pkg/factory/okta_factory.go new file mode 100644 index 00000000000..2ddd82ea686 --- /dev/null +++ b/pkg/factory/okta_factory.go @@ -0,0 +1,45 @@ +package factory + +import ( + gothOkta "github.com/markbates/goth/providers/okta" + "go.uber.org/zap" + + "github.com/transcom/mymove/pkg/handlers/authentication/okta" +) + +type ProviderConfig struct { + Name string + OrgURL string + CallbackURL string + ClientID string + Secret string + Scope []string + Logger *zap.Logger +} + +func NewProviderFactory(config ProviderConfig) (*okta.Provider, error) { + // Create a new Okta provider with goth + provider := gothOkta.New(config.ClientID, config.Secret, config.OrgURL, config.CallbackURL, config.Scope...) + // Set the name in goth + provider.SetName(config.Name) + + // Return the gothOkta provider wrapped with out our own provider struct + return okta.WrapOktaProvider(provider, config.OrgURL, config.ClientID, config.Secret, config.CallbackURL, config.Logger), nil +} + +func DummyProviderFactory(name string) (*okta.Provider, error) { + logger, _ := zap.NewDevelopment() + + // TODO: replace with consts + dummyConfig := ProviderConfig{ + Name: name, + OrgURL: "https://dummy.okta.com", + CallbackURL: "https://dummy-callback.com", + ClientID: "dummyClientID", + Secret: "dummySecret", + Scope: []string{"openid", "profile", "email"}, + Logger: logger, + } + + return NewProviderFactory(dummyConfig) +} diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index 8622ab189f6..f7ed8291509 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -630,11 +630,16 @@ func (h RedirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx.Logger().Info("User has been redirected", zap.Any("redirectURL", loginData.RedirectURL)) } +type HTTPClient interface { + Do(req *http.Request) (*http.Response, error) +} + // CallbackHandler processes a callback from okta.mil type CallbackHandler struct { Context handlers.HandlerConfig - sender notifications.NotificationSender + sender notifications.NotificationSender + HTTPClient HTTPClient } // NewCallbackHandler creates a new CallbackHandler @@ -691,6 +696,15 @@ func invalidPermissionsResponse(appCtx appcontext.AppContext, handlerConfig hand http.Redirect(w, r, landingURL.String(), http.StatusTemporaryRedirect) } +type MockHTTPClient struct { + Response *http.Response + Err error +} + +func (m *MockHTTPClient) Do(_ *http.Request) (*http.Response, error) { + return m.Response, m.Err +} + // AuthorizationCallbackHandler handles the callback from the Okta.mil authorization flow func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { appCtx := h.AppContextFromRequest(r) @@ -789,7 +803,7 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Exchange code received from login for access token. This is used during the grant_type auth flow - exchange, err := exchangeCode(r.URL.Query().Get("code"), r, appCtx, *provider) + exchange, err := exchangeCode(r.URL.Query().Get("code"), r, appCtx, *provider, h.HTTPClient) // Double error check if exchange.Error != "" { fmt.Println(exchange.Error) @@ -809,7 +823,6 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { http.Error(w, http.StatusText(500), http.StatusInternalServerError) return } - // Assign token values to session appCtx.Session().IDToken = exchange.IDToken appCtx.Session().AccessToken = exchange.AccessToken @@ -822,21 +835,16 @@ func (h CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - // ! Continuing with sessions - // TODO: convert profiledata into struct. Previous implementation used goth.User - appCtx.Session().IDToken = exchange.IDToken appCtx.Session().Email = profileData["email"] appCtx.Session().ClientID = profileData["aud"] appCtx.Logger().Info("New Login", zap.String("Okta user", profileData["preferred_username"]), zap.String("Okta email", profileData["email"]), zap.String("Host", appCtx.Session().Hostname)) - // ! Hard coded error auth result. This is because sessions are TODO - // TODO: Implement sessions and remove hard coded auth result error - // ! This will fail for now - result := AuthorizationResult(2) - dump := authorizeUser(r.Context(), appCtx, goth.User{}, sessionManager, h.sender) - appCtx.Logger().Info("Dumping var", zap.Any("dump", dump)) + // TODO: Replace with sessions' OktaUserData. Remember to update the test mock endpoints! + result := authorizeUser(r.Context(), appCtx, goth.User{ + UserID: profileData["sub"], + }, sessionManager, h.sender) switch result { case authorizationResultError: http.Error(w, http.StatusText(500), http.StatusInternalServerError) diff --git a/pkg/handlers/authentication/auth_test.go b/pkg/handlers/authentication/auth_test.go index d026455f462..2c52313993d 100644 --- a/pkg/handlers/authentication/auth_test.go +++ b/pkg/handlers/authentication/auth_test.go @@ -1,23 +1,27 @@ package authentication import ( + "bytes" "context" "encoding/gob" "fmt" + "io" "net/http" "net/http/httptest" "net/url" "strconv" "testing" + "time" "github.com/alexedwards/scs/v2" "github.com/go-chi/chi/v5" "github.com/gofrs/uuid" + "github.com/golang-jwt/jwt/v4" + "github.com/jarcoal/httpmock" "github.com/markbates/goth" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "go.uber.org/zap" - "golang.org/x/oauth2" "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/factory" @@ -35,6 +39,32 @@ import ( // UserSessionCookieName is the key suffix at which we're storing our token cookie const UserSessionCookieName = "session_token" +// This is a dumy private key that has no use and is not reflective of any real keys utilized. This key was generated +// specifically for the purpose of testing. +const DummyRSAPrivateKey = `-----BEGIN RSA PRIVATE KEY----- +MIICXQIBAAKBgQDQ62hDHRRAduSuUQDxixn61bbRLj9iBBmRG03rW3PNnkSzrcof +9ytnKY2LX2DAPaSr/1Em7fvqiovzVg43ElfFHJBrCskJqWLphifv6qoGX1pwsPA/ +Rb+MBqftMU1Zq7UC9Eis/Sje2QGx7k02JoQy+R/EP/kQq1B0p4/qCtR73QIDAQAB +AoGBALVzP+LKZsR2frdHc2JWRgIti9KyMCqZFPuKk2pOy41SYKkNz/djXTcESAM8 +m3NcFqGr5nfBSoKyQkrd+wqpy7+8X15MpClVErfUeowoOpaFQBr0E5Yf8WuzWXV2 +Daex1aeA+69OAPmYEiVJD4qY6m8vxHZZT0ISNEIW4ObhyQmhAkEA9SvhOADfQLp8 +7vZXTWW/fhapi8NiKW8cWT4wDQhwnW2glGxyVJBWwj+VtcJ8j5mEfm6vInh7QAYl +2dV/sMaNNwJBANolofvurHjd56WcdHENctAJTxiWtTqA9RIrtIIzJW7cqR4ujQKL +ndD5v2nG+b2JdlcOBzNs0LVF+ItwYMTYKYsCQQC1LqhR6tMR0r9hGUuLNxY86CKD +1vBEDoi0qvB3sTUIImv5Q+t58vEqvDK3D/Nda+YuST3EC6WJuwFd6hljWlghAkBL +s9mVywrxWtijoTrLbMZWKZTYTJyRs+TYLHCU6ljoMw1BWxg2NOtMdQ8XDyTlwIlf +xo97Khz3e1O4WARM61LnAkAzTxo/AOHVKawAR45eq4rjz0rxyCgtcTGa1qaEt9Ap +WjqcmKEkxqxz6lX/Pj2GbyikMkDThcp1bd1DRSUDOxHP +-----END RSA PRIVATE KEY----- +` +const DummyRSAPublicKey = `-----BEGIN PUBLIC KEY----- +MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDQ62hDHRRAduSuUQDxixn61bbR +Lj9iBBmRG03rW3PNnkSzrcof9ytnKY2LX2DAPaSr/1Em7fvqiovzVg43ElfFHJBr +CskJqWLphifv6qoGX1pwsPA/Rb+MBqftMU1Zq7UC9Eis/Sje2QGx7k02JoQy+R/E +P/kQq1B0p4/qCtR73QIDAQAB +-----END PUBLIC KEY----- +` + // SessionCookieName returns the session cookie name func SessionCookieName(session *auth.Session) string { return fmt.Sprintf("%s_%s", string(session.ApplicationName), UserSessionCookieName) @@ -665,10 +695,18 @@ func (suite *AuthSuite) TestRedirectLoginGovErrorMsg() { suite.Equal(authorizationResultAuthorized, result) + // Set up mock callback handler for testing h := CallbackHandler{ authContext, handlerConfig, setUpMockNotificationSender(), + &MockHTTPClient{ + Response: &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader([]byte(`{"access_token": "mockToken", "id_token": "mockIDToken"}`))), + }, + Err: nil, + }, } rr2 := httptest.NewRecorder() @@ -693,65 +731,16 @@ func (suite *AuthSuite) TestRedirectLoginGovErrorMsg() { suite.Equal(u.String(), rr2.Result().Header.Get("Location")) } -type stubLoginGovProvider struct { - StubName string - StubUser goth.User - StubSession goth.Session - StubToken string - StubClientKey string - StubState string - StubDebug bool -} - -func (s *stubLoginGovProvider) Name() string { - return s.StubName -} - -func (s *stubLoginGovProvider) SetName(name string) { - s.StubName = name -} - -func (s *stubLoginGovProvider) BeginAuth(state string) (goth.Session, error) { - s.StubState = state - return s.StubSession, nil -} - -func (s *stubLoginGovProvider) FetchUser(goth.Session) (goth.User, error) { - return s.StubUser, nil -} - -func (s *stubLoginGovProvider) FetchUserAndIDTokenByCode(_ string) (goth.User, string, error) { - return s.StubUser, s.StubToken, nil -} - -func (s *stubLoginGovProvider) ClientKey() string { - return s.StubClientKey -} - -func (s *stubLoginGovProvider) UnmarshalSession(_ string) (goth.Session, error) { - return nil, http.ErrHijacked -} -func (s *stubLoginGovProvider) Debug(setting bool) { - s.StubDebug = setting -} - -// Get new access token based on the refresh token -func (s *stubLoginGovProvider) RefreshToken(_ string) (*oauth2.Token, error) { - return nil, http.ErrHijacked -} - -// Refresh token is provided by auth provider or not -func (s *stubLoginGovProvider) RefreshTokenAvailable() bool { - return false -} - -// test to make sure the full auth flow works, although we are using -// the stubLoginGovProvider from above +// Test to make sure the full auth flow works, although we are using mock Okta endpoints func (suite *AuthSuite) TestRedirectFromLoginGovForValidUser() { + // build a real office user tioOfficeUser := factory.BuildOfficeUserWithRoles(suite.DB(), factory.GetTraitActiveOfficeUser(), []roles.RoleType{roles.RoleTypeTIO}) + // Mock the necessary Okta endpoints + mockAndActivateOktaEndpoints(tioOfficeUser) + handlerConfig := suite.HandlerConfig() appnames := handlerConfig.AppNames() @@ -762,7 +751,7 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForValidUser() { Hostname: appnames.OfficeServername, } - // login.gov state cookie + // okta.mil state cookie stateValue := "someStateValue" cookieName := StateCookieName(&session) cookie := http.Cookie{ @@ -780,20 +769,26 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForValidUser() { sessionManager := handlerConfig.SessionManagers().Office req = suite.SetupSessionRequest(req, &session, sessionManager) - stubOfficeProvider := stubLoginGovProvider{ - StubName: officeProviderName, - StubToken: "stubToken", - StubUser: goth.User{ - UserID: tioOfficeUser.User.LoginGovUUID.String(), - Email: tioOfficeUser.Email, - }, - } defer goth.ClearProviders() - goth.UseProviders(&stubOfficeProvider) + factoryProvider, err := factory.DummyProviderFactory(officeProviderName) + suite.NoError(err) + goth.UseProviders(factoryProvider) + // TODO: Consts + mockIDToken, err := generateJWTToken("dummyClientID", "https://dummy.okta.com/oauth2/default", stateValue) + suite.NoError(err) + responseBody := fmt.Sprintf(`{"access_token": "mockToken", "id_token": "%s"}`, mockIDToken) + // Create the callbackhandler with mock http client for testing h := CallbackHandler{ authContext, handlerConfig, setUpMockNotificationSender(), + &MockHTTPClient{ + Response: &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader([]byte(responseBody))), + }, + Err: nil, + }, } rr := httptest.NewRecorder() @@ -805,13 +800,80 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForValidUser() { rr.Result().Header.Get("Location")) } -// test to make sure the full auth flow works, although we are using -// the stubLoginGovProvider from above +func generateJWTToken(aud, iss, nonce string) (string, error) { + + claims := jwt.MapClaims{ + "aud": aud, + "iss": iss, + "exp": time.Now().Add(time.Hour * 72).Unix(), + "iat": time.Now().Unix(), + "nonce": nonce, + } + + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + // TODO: replace with const, it must match the okta mocks for successful auth. + token.Header["kid"] = "keyID" + + privateKey, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(DummyRSAPrivateKey)) + if err != nil { + return "", err + } + + signedToken, err := token.SignedString(privateKey) + if err != nil { + return "", err + } + + return signedToken, nil +} + +// Generate and activate Okta endpoints that will be using during the auth handlers. +func mockAndActivateOktaEndpoints(tioOfficeUser models.OfficeUser) { + // Mock the OIDC .well-known openid-configuration endpoint + // TODO: replace urls with consts and use within provider factory as well + httpmock.RegisterResponder("GET", "https://dummy.okta.com/oauth2/default/.well-known/openid-configuration", + httpmock.NewStringResponder(200, `{ + "jwks_uri": "https://dummy.okta.com/oauth2/default/.well-known/jwks.json" + }`)) + + // Mock the JWKS endpoint to receive keys for JWT verification + // TODO: replace keys with consts + httpmock.RegisterResponder("GET", "https://dummy.okta.com/oauth2/default/.well-known/jwks.json", + httpmock.NewStringResponder(200, `{ + "keys": [ + { + "alg": "RS256", + "kty": "RSA", + "use": "sig", + "n": "0OtoQx0UQHbkrlEA8YsZ-tW20S4_YgQZkRtN61tzzZ5Es63KH_crZymNi19gwD2kq_9RJu376oqL81YONxJXxRyQawrJCali6YYn7-qqBl9acLDwP0W_jAan7TFNWau1AvRIrP0o3tkBse5NNiaEMvkfxD_5EKtQdKeP6grUe90", + "e": "AQAB", + "kid": "keyID" + } + ] + }`)) + + // Mock the userinfo endpoint + // TODO: Replace with factory user information + // Sub is the Okta user ID, it is not a UUID. + tioOfficeOktaUserID := tioOfficeUser.User.LoginGovUUID.String() + httpmock.RegisterResponder("GET", "https://dummy.okta.com/oauth2/default/v1/userinfo", + httpmock.NewStringResponder(200, fmt.Sprintf(`{ + "sub": "%s", + "name": "name", + "email": "name@okta.com" + }`, tioOfficeOktaUserID))) + + httpmock.Activate() +} + +// Test to make sure the full auth flow works, although we are using mock Okta endpoints func (suite *AuthSuite) TestRedirectFromLoginGovForInvalidUser() { - // build a real office user tioOfficeUser := factory.BuildOfficeUserWithRoles(suite.DB(), nil, []roles.RoleType{roles.RoleTypeTIO}) suite.False(tioOfficeUser.Active) + // Mock the necessary Okta endpoints + mockAndActivateOktaEndpoints(tioOfficeUser) + handlerConfig := suite.HandlerConfig() appnames := handlerConfig.AppNames() @@ -822,8 +884,12 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForInvalidUser() { Hostname: appnames.OfficeServername, } - // login.gov state cookie + // okta.mil state cookie stateValue := "someStateValue" + // The code value is what is used to retrieve the exchange token frin the code passed through the URL. As long as the code value exists, + // then the exchangeCode function will not fail. HOWEVER, we still need to provide a mock exchange token in the body so that it + // can be verified and a user can be pulled from it. + codeValue := "someCodeValue" cookieName := StateCookieName(&session) cookie := http.Cookie{ Name: cookieName, @@ -831,8 +897,8 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForInvalidUser() { Path: "/", Expires: auth.GetExpiryTimeFromMinutes(auth.SessionExpiryInMinutes), } - req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/login-gov/callback?state=%s", - appnames.OfficeServername, stateValue), nil) + req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/login-gov/callback?state=%s&code=%s", + appnames.OfficeServername, stateValue, codeValue), nil) req.AddCookie(&cookie) authContext := suite.AuthContext() @@ -840,20 +906,26 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForInvalidUser() { sessionManager := handlerConfig.SessionManagers().Office req = suite.SetupSessionRequest(req, &session, sessionManager) - stubOfficeProvider := stubLoginGovProvider{ - StubName: officeProviderName, - StubToken: "stubToken", - StubUser: goth.User{ - UserID: tioOfficeUser.User.LoginGovUUID.String(), - Email: tioOfficeUser.Email, - }, - } defer goth.ClearProviders() - goth.UseProviders(&stubOfficeProvider) + factoryProvider, err := factory.DummyProviderFactory(officeProviderName) + suite.NoError(err) + goth.UseProviders(factoryProvider) + // TODO: Replace with consts + mockIDToken, err := generateJWTToken("dummyClientID", "https://dummy.okta.com/oauth2/default", stateValue) + suite.NoError(err) + responseBody := fmt.Sprintf(`{"access_token": "mockToken", "id_token": "%s"}`, mockIDToken) + // Create the callbackhandler with mock http client for testing h := CallbackHandler{ authContext, handlerConfig, setUpMockNotificationSender(), + &MockHTTPClient{ + Response: &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader([]byte(responseBody))), + }, + Err: nil, + }, } rr := httptest.NewRecorder() diff --git a/pkg/handlers/authentication/okta/provider.go b/pkg/handlers/authentication/okta/provider.go index ce7ae1867ad..df135ee92b2 100644 --- a/pkg/handlers/authentication/okta/provider.go +++ b/pkg/handlers/authentication/okta/provider.go @@ -77,33 +77,6 @@ func ConvertGothProviderToOktaProvider(gothProvider goth.Provider) (*Provider, e return provider, nil } -// func getProviderName(r *http.Request) string { -// session := auth.SessionFromRequestContext(r) - -// // Set the provider name based on of it is an office or admin app. Remember, the provider is slected by its name -// if session.IsOfficeApp() { -// return OfficeProviderName -// } else if session.IsAdminApp() { -// return AdminProviderName -// } -// return MilProviderName -// } - -// ! This func will likely come back during continuation of the sessions story -// // This function will return the ClientID of the current provider -// func (op *OktaProvider) ClientID(r *http.Request) (string, error) { -// // Default the provider name to the "MilProviderName" which is the customer application -// providerName := getProviderName(r) - -// // Retrieve the provider based on its name -// gothProvider, err := goth.GetProvider(providerName) -// if err != nil { -// return "", err -// } - -// return gothProvider.ClientKey, nil -// } - // This function will use the OktaProvider to return the correct authorization URL to use func (op *Provider) AuthorizationURL(r *http.Request) (*Data, error) { @@ -158,10 +131,14 @@ func NewOktaProvider(logger *zap.Logger) *Provider { // This function allows us to wrap new registered providers with the zap logger. The initial Okta provider is already wrapped // This will wrap the gothOkta provider with our own version of OktaProvider (With added methods) -func wrapOktaProvider(provider *gothOkta.Provider, logger *zap.Logger) *Provider { +func WrapOktaProvider(provider *gothOkta.Provider, orgURL string, clientID string, secret string, callbackURL string, logger *zap.Logger) *Provider { return &Provider{ - Provider: *provider, - logger: logger, + Provider: *provider, + orgURL: orgURL, + clientID: clientID, + secret: secret, + callbackURL: callbackURL, + logger: logger, } } @@ -211,18 +188,11 @@ func (op *Provider) RegisterOktaProvider(name string, orgURL string, callbackURL provider := gothOkta.New(clientID, secret, orgURL, callbackURL, scope...) // Set the name manualy provider.SetName(name) - // Wrap - wrap := wrapOktaProvider(provider, op.logger) - // Set hostname - wrap.SetOrgURL(orgURL) - wrap.SetClientID(clientID) - wrap.SetSecret(secret) - wrap.SetCallbackURL(callbackURL) - // Assign to the active goth providers - goth.UseProviders(wrap) + // Assign to the active goth providers in a type asserted format based on our Provider struct + goth.UseProviders(WrapOktaProvider(provider, orgURL, clientID, secret, callbackURL, op.logger)) // Check that the provider exists now. The previous functions do not have error handling - err := verifyProvider(name) + err := VerifyProvider(name) if err != nil { op.logger.Error("Could not verify goth provider", zap.Error(err)) return err @@ -231,7 +201,7 @@ func (op *Provider) RegisterOktaProvider(name string, orgURL string, callbackURL } // Check if the provided provider name exists -func verifyProvider(name string) error { +func VerifyProvider(name string) error { provider, err := goth.GetProvider(name) fmt.Println(provider) if err != nil { diff --git a/pkg/handlers/authentication/okta/provider_test.go b/pkg/handlers/authentication/okta/provider_test.go new file mode 100644 index 00000000000..93bc6af3ec5 --- /dev/null +++ b/pkg/handlers/authentication/okta/provider_test.go @@ -0,0 +1,35 @@ +package okta_test + +import ( + "testing" + + "github.com/markbates/goth" + "github.com/stretchr/testify/suite" + + "github.com/transcom/mymove/pkg/handlers" + "github.com/transcom/mymove/pkg/handlers/authentication/okta" + "github.com/transcom/mymove/pkg/notifications" + "github.com/transcom/mymove/pkg/testingsuite" +) + +type OktaSuite struct { + handlers.BaseHandlerTestSuite +} + +func TestAuthSuite(t *testing.T) { + hs := &OktaSuite{ + BaseHandlerTestSuite: handlers.NewBaseHandlerTestSuite(notifications.NewStubNotificationSender("milmovelocal"), testingsuite.CurrentPackage(), testingsuite.WithPerTestTransaction()), + } + suite.Run(t, hs) + hs.PopTestSuite.TearDown() +} + +// TODO: Flesh out with actually accessing the data. See error that can pop up when okta provider is not wrapped correctly. +func (suite *OktaSuite) TestConvertGothProviderToOktaProvider() { + oktaProvider := &okta.Provider{} + gothProvider := goth.Provider(oktaProvider) + + provider, err := okta.ConvertGothProviderToOktaProvider(gothProvider) + suite.NoError(err) + suite.Equal(oktaProvider, provider) +} diff --git a/pkg/handlers/authentication/okta_auth_code_flow.go b/pkg/handlers/authentication/okta_auth_code_flow.go index d961f3018c8..2aea91ba175 100644 --- a/pkg/handlers/authentication/okta_auth_code_flow.go +++ b/pkg/handlers/authentication/okta_auth_code_flow.go @@ -33,13 +33,18 @@ func getProfileData(appCtx appcontext.AppContext, provider okta.Provider) (map[s h.Add("Accept", "application/json") client := &http.Client{} - resp, _ := client.Do(req) - body, _ := io.ReadAll(resp.Body) + resp, err := client.Do(req) + if err != nil { + appCtx.Logger().Error("could not execute request", zap.Error(err)) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + appCtx.Logger().Error("could not read response body", zap.Error(err)) + } defer resp.Body.Close() - err := json.Unmarshal(body, &m) + err = json.Unmarshal(body, &m) if err != nil { appCtx.Logger().Error("could not unmarshal body", zap.Error(err)) - // No return is intentional } return m, nil @@ -68,9 +73,7 @@ func verifyToken(t string, nonce string, provider okta.Provider) (*verifier.Jwt, return nil, fmt.Errorf("token could not be verified: %s", "") } -// ! Refactor once chamber is holding new secrets -func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext, provider okta.Provider) (Exchange, error) { - +func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext, provider okta.Provider, client HTTPClient) (Exchange, error) { authHeader := base64.StdEncoding.EncodeToString( []byte(provider.GetClientID() + ":" + provider.GetSecret())) @@ -89,7 +92,6 @@ func exchangeCode(code string, r *http.Request, appCtx appcontext.AppContext, pr h.Add("Connection", "close") h.Add("Content-Length", "0") - client := &http.Client{} resp, err := client.Do(req) if err != nil { appCtx.Logger().Error("Code exchange", zap.Error(err)) From 51fcda5f6b70035d242f5ba97cca40469ab1e63c Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Wed, 16 Aug 2023 14:28:29 +0000 Subject: [PATCH 19/24] Removed logingov flags and replaced with okta. Redirected login-gov to okta for internal routes. --- .envrc | 6 +- cmd/milmove/serve.go | 17 ++-- pkg/cli/auth.go | 103 +++++++-------------- pkg/handlers/authentication/auth.go | 1 + pkg/handlers/authentication/auth_test.go | 8 +- pkg/handlers/authentication/login_gov.go | 2 +- pkg/handlers/routing/routing_init.go | 4 +- pkg/handlers/routing/routing_init_test.go | 2 +- src/containers/Headers/LoggedOutHeader.jsx | 2 +- src/containers/LoginButton/LoginButton.jsx | 2 +- src/pages/SignIn/SignIn.jsx | 2 +- 11 files changed, 54 insertions(+), 95 deletions(-) diff --git a/.envrc b/.envrc index 144beb2eb64..b941647dee5 100644 --- a/.envrc +++ b/.envrc @@ -140,17 +140,17 @@ export OKTA_TENANT_CALLBACK_PROTOCOL=https # Customer require OKTA_CUSTOMER_SECRET_KEY "See 'DISABLE_AWS_VAULT_WRAPPER=1 AWS_REGION=us-gov-west-1 aws-vault exec transcom-gov-dev -- chamber read app-devlocal okta-customer-secret-key'" export OKTA_CUSTOMER_CLIENT_ID=0oa3jalqz3iCyRT9i0k6 -export OKTA_CUSTOMER_CALLBACK_URL=http://milmovelocal:3000/auth/login-gov/callback +export OKTA_CUSTOMER_CALLBACK_URL=http://milmovelocal:3000/auth/okta/callback # Office require OKTA_OFFICE_SECRET_KEY "See 'DISABLE_AWS_VAULT_WRAPPER=1 AWS_REGION=us-gov-west-1 aws-vault exec transcom-gov-dev -- chamber read app-devlocal okta-office-secret-key'" export OKTA_OFFICE_CLIENT_ID=0oa3j8zgqkIUDrhOy0k6 -export OKTA_OFFICE_CALLBACK_URL=http://officelocal:3000/auth/login-gov/callback +export OKTA_OFFICE_CALLBACK_URL=http://officelocal:3000/auth/okta/callback # Admin require OKTA_ADMIN_SECRET_KEY "See 'DISABLE_AWS_VAULT_WRAPPER=1 AWS_REGION=us-gov-west-1 aws-vault exec transcom-gov-dev -- chamber read app-devlocal okta-admin-secret-key'" export OKTA_ADMIN_CLIENT_ID=0oa3j9kbaTicDz0az0k6 -export OKTA_ADMIN_CALLBACK_URL=http://adminlocal:3000/auth/login-gov/callback +export OKTA_ADMIN_CALLBACK_URL=http://adminlocal:3000/auth/okta/callback # JSON Web Token (JWT) config CLIENT_AUTH_SECRET_KEY=$(cat config/tls/devlocal-client_auth_secret.key) diff --git a/cmd/milmove/serve.go b/cmd/milmove/serve.go index d933aff728a..64588bd99ba 100644 --- a/cmd/milmove/serve.go +++ b/cmd/milmove/serve.go @@ -428,21 +428,18 @@ func buildRoutingConfig(appCtx appcontext.AppContext, v *viper.Viper, redisPool } clientAuthSecretKey := v.GetString(cli.ClientAuthSecretKeyFlag) - loginGovCallbackProtocol := v.GetString(cli.LoginGovCallbackProtocolFlag) - loginGovCallbackPort := v.GetInt(cli.LoginGovCallbackPortFlag) - loginGovSecretKey := v.GetString(cli.LoginGovSecretKeyFlag) - loginGovHostname := v.GetString(cli.LoginGovHostnameFlag) + + callbackProtocol := v.GetString(cli.OktaTenantCallbackProtocolFlag) + callbackPort := v.GetInt(cli.OktaTenantCallbackPortFlag) + orgURL := v.GetString(cli.OktaTenantOrgURLFlag) // Assert that our secret keys can be parsed into actual private keys // TODO: Store the parsed key in handlers/AppContext instead of parsing every time - if _, parseRSAPrivateKeyFromPEMErr := jwt.ParseRSAPrivateKeyFromPEM([]byte(loginGovSecretKey)); parseRSAPrivateKeyFromPEMErr != nil { - appCtx.Logger().Fatal("Login.gov private key", zap.Error(parseRSAPrivateKeyFromPEMErr)) - } if _, parseRSAPrivateKeyFromPEMErr := jwt.ParseRSAPrivateKeyFromPEM([]byte(clientAuthSecretKey)); parseRSAPrivateKeyFromPEMErr != nil { appCtx.Logger().Fatal("Client auth private key", zap.Error(parseRSAPrivateKeyFromPEMErr)) } - if len(loginGovHostname) == 0 { - appCtx.Logger().Fatal("Must provide the Login.gov hostname parameter, exiting") + if len(orgURL) == 0 { + appCtx.Logger().Fatal("Must provide the okta.mil orgURL parameter, exiting") } // Register Okta authentication provider for My.(move.mil) @@ -453,7 +450,7 @@ func buildRoutingConfig(appCtx appcontext.AppContext, v *viper.Viper, redisPool } // TODO: Update loginGov callbacks to Okta - routingConfig.AuthContext = authentication.NewAuthContext(appCtx.Logger(), *oktaProvider, loginGovCallbackProtocol, loginGovCallbackPort) + routingConfig.AuthContext = authentication.NewAuthContext(appCtx.Logger(), *oktaProvider, callbackProtocol, callbackPort) // Email notificationSender, err := notifications.InitEmail(v, appCtx.Logger()) diff --git a/pkg/cli/auth.go b/pkg/cli/auth.go index 4926d79406a..8818f87348b 100644 --- a/pkg/cli/auth.go +++ b/pkg/cli/auth.go @@ -21,23 +21,8 @@ const ( //RA Validator: jneuner@mitre.org //RA Modified Severity: CAT III // #nosec G101 - // ClientAuthSecretKeyFlag is the Client Auth Secret Key Flag ClientAuthSecretKeyFlag string = "client-auth-secret-key" - // LoginGovCallbackProtocolFlag is the Login.gov Callback Protocol Flag - LoginGovCallbackProtocolFlag string = "login-gov-callback-protocol" - // LoginGovCallbackPortFlag is the Login.gov Callback Port Flag - LoginGovCallbackPortFlag string = "login-gov-callback-port" - // LoginGovSecretKeyFlag is the Login.gov Secret Key Flag - LoginGovSecretKeyFlag string = "login-gov-secret-key" - // LoginGovMyClientIDFlag is the Login.gov My Client ID Flag - LoginGovMyClientIDFlag string = "login-gov-my-client-id" - // LoginGovOfficeClientIDFlag is the Login.gov Office Client ID Flag - LoginGovOfficeClientIDFlag string = "login-gov-office-client-id" - // LoginGovAdminClientIDFlag is the Login.gov Admin Client ID Flag - LoginGovAdminClientIDFlag string = "login-gov-admin-client-id" - // LoginGovHostnameFlag is the Login.gov Hostname Flag - LoginGovHostnameFlag string = "login-gov-hostname" // Okta flags for local development environment that serves test-milmove.okta.mil // Okta tenant flags @@ -105,81 +90,57 @@ func (e *errInvalidClientID) Error() string { func InitAuthFlags(flag *pflag.FlagSet) { flag.String(ClientAuthSecretKeyFlag, "", "Client auth secret JWT key.") - flag.String(LoginGovCallbackProtocolFlag, "https", "Protocol for non local environments.") - flag.Int(LoginGovCallbackPortFlag, 443, "The port for callback urls.") - flag.String(LoginGovSecretKeyFlag, "", "Login.gov auth secret JWT key.") - flag.String(LoginGovMyClientIDFlag, "", "Client ID registered with login gov.") - flag.String(LoginGovOfficeClientIDFlag, "", "Client ID registered with login gov.") - flag.String(LoginGovAdminClientIDFlag, "", "Client ID registered with login gov.") - flag.String(LoginGovHostnameFlag, "secure.login.gov", "Hostname for communicating with login gov.") - - // TODO: Replace Okta os.Getenv - - // // Okta flags - // flag.String(OktaTenantIssuerURLFlag, os.Getenv("OKTA_TENANT_ORG_URL"), "Okta tenant issuer URL.") - // flag.Int(OktaTenantCallbackPortFlag, 443, "Okta tenant callback port.") - - // // Customer flags - // flag.String(OktaCustomerSecretKeyFlag, os.Getenv("OKTA_CUSTOMER_SECRET_KEY"), "Okta customer secret key.") - // flag.String(OktaCustomerClientIDFlag, os.Getenv("OKTA_CUSTOMER_CLIENT_ID"), "Okta customer client ID.") - // flag.String(OktaCustomerHostnameFlag, os.Getenv("OKTA_CUSTOMER_HOSTNAME"), "Okta customer hostname.") - // flag.String(OktaCustomerCallbackProtocolFlag, os.Getenv("OKTA_CUSTOMER_CALLBACK_PROTOCOL"), "Okta customer callback protocol.") - - // // Office flags - // flag.String(OktaOfficeSecretKeyFlag, os.Getenv("OKTA_OFFICE_SECRET_KEY"), "Okta office secret key.") - // flag.String(OktaOfficeClientIDFlag, os.Getenv("OKTA_OFFICE_CLIENT_ID"), "Okta office client ID.") - // flag.String(OktaOfficeHostnameFlag, os.Getenv("OKTA_OFFICE_HOSTNAME"), "Okta office hostname.") - // flag.String(OktaOfficeCallbackProtocolFlag, os.Getenv("OKTA_OFFICE_CALLBACK_PROTOCOL"), "Okta office callback protocol.") - - // // Admin flags - // flag.String(OktaAdminSecretKeyFlag, os.Getenv("OKTA_ADMIN_SECRET_KEY"), "Okta admin secret key.") - // flag.String(OktaAdminClientIDFlag, os.Getenv("OKTA_ADMIN_CLIENT_ID"), "Okta admin client ID.") - // flag.String(OktaAdminHostnameFlag, os.Getenv("OKTA_ADMIN_HOSTNAME"), "Okta admin hostname.") - // flag.String(OktaAdminCallbackProtocolFlag, os.Getenv("OKTA_ADMIN_CALLBACK_PROTOCOL"), "Okta admin callback protocol.") + flag.String(OktaTenantOrgURLFlag, "", "Okta tenant org URL.") + flag.Int(OktaTenantCallbackPortFlag, 443, "The port for callback URLs.") + flag.String(OktaTenantCallbackProtocolFlag, "https", "Protocol for non local environments.") + flag.String(OktaCustomerClientIDFlag, "", "The client ID for the military customer app, aka 'my'.") + flag.String(OktaCustomerCallbackURL, "", "The callback URL from logging in to the customer Okta app back to MilMove.") + flag.String(OktaCustomerSecretKeyFlag, "", "The secret key for the miltiary customer app, aka 'my'.") + flag.String(OktaOfficeClientIDFlag, "", "The client ID for the military Office app, aka 'my'.") + flag.String(OktaOfficeCallbackURL, "", "The callback URL from logging in to the office Okta app back to MilMove.") + flag.String(OktaOfficeSecretKeyFlag, "", "The secret key for the miltiary Office app, aka 'my'.") + flag.String(OktaAdminClientIDFlag, "", "The client ID for the military Admin app, aka 'my'.") + flag.String(OktaAdminCallbackURL, "", "The callback URL from logging in to the admin Okta app back to MilMove.") + flag.String(OktaAdminSecretKeyFlag, "", "The secret key for the miltiary Admin app, aka 'my'.") } // CheckAuth validates Auth command line flags func CheckAuth(v *viper.Viper) error { - if err := ValidateProtocol(v, LoginGovCallbackProtocolFlag); err != nil { + if err := ValidateProtocol(v, OktaTenantCallbackProtocolFlag); err != nil { return err } - if err := ValidateHost(v, LoginGovHostnameFlag); err != nil { + if err := ValidatePort(v, OktaTenantCallbackPortFlag); err != nil { return err } - secureLoginGov := "secure.login.gov" - sandboxLoginGov := "idp.int.identitysandbox.gov" - if loginGovHostname := v.GetString(LoginGovHostnameFlag); loginGovHostname != secureLoginGov && loginGovHostname != sandboxLoginGov { - return errors.Wrap(&errInvalidHost{Host: loginGovHostname}, fmt.Sprintf("%s is invalid, expected %s or %s", LoginGovHostnameFlag, secureLoginGov, sandboxLoginGov)) + clientIDVars := []string{ + OktaCustomerClientIDFlag, + OktaOfficeClientIDFlag, + OktaAdminClientIDFlag, } - if err := ValidatePort(v, LoginGovCallbackPortFlag); err != nil { - return err - } - - clientIDVars := []string{ - LoginGovMyClientIDFlag, - LoginGovOfficeClientIDFlag, - LoginGovAdminClientIDFlag, + secretKeyVars := []string{ + OktaCustomerSecretKeyFlag, + OktaOfficeSecretKeyFlag, + OktaAdminSecretKeyFlag, } for _, c := range clientIDVars { - err := ValidateClientID(v, c) - if err != nil { - return err + clientID := v.GetString(c) + { + if len(clientID) == 0 { + return errors.Errorf("%s is missing", c) + } } } - privateKey := v.GetString(LoginGovSecretKeyFlag) - if len(privateKey) == 0 { - return errors.Errorf("%s is missing", LoginGovSecretKeyFlag) - } - - keys := ParsePrivateKey(privateKey) - if len(keys) == 0 { - return errors.Errorf("%s is missing key block", LoginGovSecretKeyFlag) + for _, s := range secretKeyVars { + privateKey := v.GetString(s) + if len(privateKey) == 0 { + return errors.Errorf("%s is missing", s) + } } return nil diff --git a/pkg/handlers/authentication/auth.go b/pkg/handlers/authentication/auth.go index f7ed8291509..420aecd1fba 100644 --- a/pkg/handlers/authentication/auth.go +++ b/pkg/handlers/authentication/auth.go @@ -648,6 +648,7 @@ func NewCallbackHandler(ac Context, hc handlers.HandlerConfig, sender notificati Context: ac, HandlerConfig: hc, sender: sender, + HTTPClient: &http.Client{}, } return handler } diff --git a/pkg/handlers/authentication/auth_test.go b/pkg/handlers/authentication/auth_test.go index 2c52313993d..3db0ed8ddcd 100644 --- a/pkg/handlers/authentication/auth_test.go +++ b/pkg/handlers/authentication/auth_test.go @@ -668,7 +668,7 @@ func (suite *AuthSuite) TestRedirectLoginGovErrorMsg() { handlerConfig := suite.HandlerConfig() appnames := handlerConfig.AppNames() - req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/login-gov/callback", appnames.OfficeServername), nil) + req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/okta/callback", appnames.OfficeServername), nil) fakeToken := "some_token" session := auth.Session{ @@ -760,7 +760,7 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForValidUser() { Path: "/", Expires: auth.GetExpiryTimeFromMinutes(auth.SessionExpiryInMinutes), } - req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/login-gov/callback?state=%s", + req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/okta/callback?state=%s", appnames.OfficeServername, stateValue), nil) req.AddCookie(&cookie) @@ -897,7 +897,7 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForInvalidUser() { Path: "/", Expires: auth.GetExpiryTimeFromMinutes(auth.SessionExpiryInMinutes), } - req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/login-gov/callback?state=%s&code=%s", + req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/okta/callback?state=%s&code=%s", appnames.OfficeServername, stateValue, codeValue), nil) req.AddCookie(&cookie) @@ -1497,7 +1497,7 @@ func (suite *AuthSuite) TestLoginGovAuthenticatedRedirect() { Hostname: appnames.OfficeServername, Email: officeUser.Email, } - req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/login-gov", appnames.OfficeServername), nil) + req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/okta", appnames.OfficeServername), nil) ctx := auth.SetSessionInRequestContext(req, &session) authContext := suite.AuthContext() h := RedirectHandler{ diff --git a/pkg/handlers/authentication/login_gov.go b/pkg/handlers/authentication/login_gov.go index 36de2a00f75..4a5781b704f 100644 --- a/pkg/handlers/authentication/login_gov.go +++ b/pkg/handlers/authentication/login_gov.go @@ -103,7 +103,7 @@ func (p LoginGovProvider) getOpenIDProvider(hostname string, clientID string, ca return openidConnect.New( clientID, p.secretKey, - fmt.Sprintf("%s://%s:%d/auth/login-gov/callback", callbackProtocol, hostname, callbackPort), + fmt.Sprintf("%s://%s:%d/auth/okta/callback", callbackProtocol, hostname, callbackPort), fmt.Sprintf("https://%s/.well-known/openid-configuration", p.hostname), ) } diff --git a/pkg/handlers/routing/routing_init.go b/pkg/handlers/routing/routing_init.go index 7c98d674036..82a87781aca 100644 --- a/pkg/handlers/routing/routing_init.go +++ b/pkg/handlers/routing/routing_init.go @@ -517,8 +517,8 @@ func mountGHCAPI(appCtx appcontext.AppContext, routingConfig *Config, site chi.R func mountAuthRoutes(appCtx appcontext.AppContext, routingConfig *Config, site chi.Router) { site.Route("/auth/", func(r chi.Router) { r.Use(middleware.NoCache()) - r.Method("GET", "/login-gov", authentication.NewRedirectHandler(routingConfig.AuthContext, routingConfig.HandlerConfig, routingConfig.HandlerConfig.UseSecureCookie())) - r.Method("GET", "/login-gov/callback", authentication.NewCallbackHandler(routingConfig.AuthContext, routingConfig.HandlerConfig, routingConfig.HandlerConfig.NotificationSender())) + r.Method("GET", "/okta", authentication.NewRedirectHandler(routingConfig.AuthContext, routingConfig.HandlerConfig, routingConfig.HandlerConfig.UseSecureCookie())) + r.Method("GET", "/okta/callback", authentication.NewCallbackHandler(routingConfig.AuthContext, routingConfig.HandlerConfig, routingConfig.HandlerConfig.NotificationSender())) r.Method("POST", "/logout", authentication.NewLogoutHandler(routingConfig.AuthContext, routingConfig.HandlerConfig)) }) diff --git a/pkg/handlers/routing/routing_init_test.go b/pkg/handlers/routing/routing_init_test.go index e28989888a1..27c709e784c 100644 --- a/pkg/handlers/routing/routing_init_test.go +++ b/pkg/handlers/routing/routing_init_test.go @@ -172,7 +172,7 @@ func (suite *RoutingSuite) TestBasicAuthLoginRouting() { serviceMember := factory.BuildServiceMember(suite.DB(), factory.GetTraitActiveServiceMemberUser(), nil) // an authenticted request will redirect and we just want to check // that the route is set up correctly - req := suite.NewAuthenticatedMilRequest("GET", "/auth/login-gov", nil, serviceMember) + req := suite.NewAuthenticatedMilRequest("GET", "/auth/okta", nil, serviceMember) rr := httptest.NewRecorder() siteHandler.ServeHTTP(rr, req) suite.Equal(http.StatusTemporaryRedirect, rr.Code) diff --git a/src/containers/Headers/LoggedOutHeader.jsx b/src/containers/Headers/LoggedOutHeader.jsx index 0749e2a0783..25b865ade1c 100644 --- a/src/containers/Headers/LoggedOutHeader.jsx +++ b/src/containers/Headers/LoggedOutHeader.jsx @@ -12,7 +12,7 @@ const LoggedOutHeader = () => { { - window.location.href = '/auth/login-gov'; + window.location.href = '/auth/okta'; }} closeModal={() => setShowEula(false)} /> diff --git a/src/containers/LoginButton/LoginButton.jsx b/src/containers/LoginButton/LoginButton.jsx index 024a1fd4a95..bfbe59f518d 100644 --- a/src/containers/LoginButton/LoginButton.jsx +++ b/src/containers/LoginButton/LoginButton.jsx @@ -26,7 +26,7 @@ const LoginButton = ({ isLoggedIn, logOut, showDevlocalButton, isProfileComplete { - window.location.href = '/auth/login-gov'; + window.location.href = '/auth/okta'; }} closeModal={() => setShowEula(false)} /> diff --git a/src/pages/SignIn/SignIn.jsx b/src/pages/SignIn/SignIn.jsx index 617d10beede..7ca4c3eedd2 100644 --- a/src/pages/SignIn/SignIn.jsx +++ b/src/pages/SignIn/SignIn.jsx @@ -38,7 +38,7 @@ const SignIn = ({ context, showLocalDevLogin }) => { { - window.location.href = '/auth/login-gov'; + window.location.href = '/auth/okta'; }} closeModal={() => setShowEula(false)} /> From 65f38e83252cd05c1f62034e79cab6fd1f9ca874 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Wed, 16 Aug 2023 15:23:51 +0000 Subject: [PATCH 20/24] Refactored factory and tests --- pkg/factory/okta_factory.go | 26 ++++--- pkg/handlers/authentication/auth_test.go | 79 ++++++++++---------- pkg/handlers/authentication/okta/provider.go | 6 ++ 3 files changed, 62 insertions(+), 49 deletions(-) diff --git a/pkg/factory/okta_factory.go b/pkg/factory/okta_factory.go index 2ddd82ea686..af614157442 100644 --- a/pkg/factory/okta_factory.go +++ b/pkg/factory/okta_factory.go @@ -7,6 +7,13 @@ import ( "github.com/transcom/mymove/pkg/handlers/authentication/okta" ) +const dummyOktaOrgURL = "https://dummy.okta.com" +const dummyOktaCallbackURL = "https://dummy.okta.com/auth/callback" +const dummyClientID = "dummyClientID" +const dummySecret = "dummySecret" + +var dummyOIDCScope = []string{"openid", "profile", "email"} + type ProviderConfig struct { Name string OrgURL string @@ -17,7 +24,7 @@ type ProviderConfig struct { Logger *zap.Logger } -func NewProviderFactory(config ProviderConfig) (*okta.Provider, error) { +func CreateAndWrapProvider(config ProviderConfig) (*okta.Provider, error) { // Create a new Okta provider with goth provider := gothOkta.New(config.ClientID, config.Secret, config.OrgURL, config.CallbackURL, config.Scope...) // Set the name in goth @@ -27,19 +34,18 @@ func NewProviderFactory(config ProviderConfig) (*okta.Provider, error) { return okta.WrapOktaProvider(provider, config.OrgURL, config.ClientID, config.Secret, config.CallbackURL, config.Logger), nil } -func DummyProviderFactory(name string) (*okta.Provider, error) { +func BuildOktaProvider(name string) (*okta.Provider, error) { logger, _ := zap.NewDevelopment() - // TODO: replace with consts - dummyConfig := ProviderConfig{ + provider := ProviderConfig{ Name: name, - OrgURL: "https://dummy.okta.com", - CallbackURL: "https://dummy-callback.com", - ClientID: "dummyClientID", - Secret: "dummySecret", - Scope: []string{"openid", "profile", "email"}, + OrgURL: dummyOktaOrgURL, + CallbackURL: dummyOktaCallbackURL, + ClientID: dummyClientID, + Secret: dummySecret, + Scope: dummyOIDCScope, Logger: logger, } - return NewProviderFactory(dummyConfig) + return CreateAndWrapProvider(provider) } diff --git a/pkg/handlers/authentication/auth_test.go b/pkg/handlers/authentication/auth_test.go index 3db0ed8ddcd..a1d7cd34626 100644 --- a/pkg/handlers/authentication/auth_test.go +++ b/pkg/handlers/authentication/auth_test.go @@ -65,6 +65,9 @@ P/kQq1B0p4/qCtR73QIDAQAB -----END PUBLIC KEY----- ` +const DummyRSAModulus = "0OtoQx0UQHbkrlEA8YsZ-tW20S4_YgQZkRtN61tzzZ5Es63KH_crZymNi19gwD2kq_9RJu376oqL81YONxJXxRyQawrJCali6YYn7-qqBl9acLDwP0W_jAan7TFNWau1AvRIrP0o3tkBse5NNiaEMvkfxD_5EKtQdKeP6grUe90" +const jwtKeyID = "keyID" + // SessionCookieName returns the session cookie name func SessionCookieName(session *auth.Session) string { return fmt.Sprintf("%s_%s", string(session.ApplicationName), UserSessionCookieName) @@ -676,7 +679,7 @@ func (suite *AuthSuite) TestRedirectLoginGovErrorMsg() { IDToken: fakeToken, Hostname: appnames.OfficeServername, } - // login.gov state cookie + // okta.mil state cookie cookieName := StateCookieName(&session) cookie := http.Cookie{ Name: cookieName, @@ -694,7 +697,6 @@ func (suite *AuthSuite) TestRedirectLoginGovErrorMsg() { &userIdentity, sessionManager) suite.Equal(authorizationResultAuthorized, result) - // Set up mock callback handler for testing h := CallbackHandler{ authContext, @@ -703,7 +705,7 @@ func (suite *AuthSuite) TestRedirectLoginGovErrorMsg() { &MockHTTPClient{ Response: &http.Response{ StatusCode: 200, - Body: io.NopCloser(bytes.NewReader([]byte(`{"access_token": "mockToken", "id_token": "mockIDToken"}`))), + Body: io.NopCloser(bytes.NewReader([]byte(`{"access_token": "mockToken", "id_token": "broken_id_token"}`))), }, Err: nil, }, @@ -738,8 +740,12 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForValidUser() { tioOfficeUser := factory.BuildOfficeUserWithRoles(suite.DB(), factory.GetTraitActiveOfficeUser(), []roles.RoleType{roles.RoleTypeTIO}) + // Build provider + provider, err := factory.BuildOktaProvider(officeProviderName) + suite.NoError(err) + // Mock the necessary Okta endpoints - mockAndActivateOktaEndpoints(tioOfficeUser) + mockAndActivateOktaEndpoints(tioOfficeUser, provider) handlerConfig := suite.HandlerConfig() appnames := handlerConfig.AppNames() @@ -770,11 +776,8 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForValidUser() { req = suite.SetupSessionRequest(req, &session, sessionManager) defer goth.ClearProviders() - factoryProvider, err := factory.DummyProviderFactory(officeProviderName) - suite.NoError(err) - goth.UseProviders(factoryProvider) - // TODO: Consts - mockIDToken, err := generateJWTToken("dummyClientID", "https://dummy.okta.com/oauth2/default", stateValue) + goth.UseProviders(provider) + mockIDToken, err := generateJWTToken(provider.GetClientID(), provider.GetIssuerURL(), stateValue) suite.NoError(err) responseBody := fmt.Sprintf(`{"access_token": "mockToken", "id_token": "%s"}`, mockIDToken) // Create the callbackhandler with mock http client for testing @@ -811,8 +814,7 @@ func generateJWTToken(aud, iss, nonce string) (string, error) { } token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) - // TODO: replace with const, it must match the okta mocks for successful auth. - token.Header["kid"] = "keyID" + token.Header["kid"] = jwtKeyID privateKey, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(DummyRSAPrivateKey)) if err != nil { @@ -828,35 +830,33 @@ func generateJWTToken(aud, iss, nonce string) (string, error) { } // Generate and activate Okta endpoints that will be using during the auth handlers. -func mockAndActivateOktaEndpoints(tioOfficeUser models.OfficeUser) { +func mockAndActivateOktaEndpoints(tioOfficeUser models.OfficeUser, provider *okta.Provider) { // Mock the OIDC .well-known openid-configuration endpoint - // TODO: replace urls with consts and use within provider factory as well - httpmock.RegisterResponder("GET", "https://dummy.okta.com/oauth2/default/.well-known/openid-configuration", - httpmock.NewStringResponder(200, `{ - "jwks_uri": "https://dummy.okta.com/oauth2/default/.well-known/jwks.json" - }`)) + jwksURL := provider.GetJWKSURL() + httpmock.RegisterResponder("GET", provider.GetOpenIDConfigURL(), + httpmock.NewStringResponder(200, fmt.Sprintf(`{ + "jwks_uri": %s + }`, jwksURL))) // Mock the JWKS endpoint to receive keys for JWT verification - // TODO: replace keys with consts - httpmock.RegisterResponder("GET", "https://dummy.okta.com/oauth2/default/.well-known/jwks.json", - httpmock.NewStringResponder(200, `{ - "keys": [ - { - "alg": "RS256", - "kty": "RSA", - "use": "sig", - "n": "0OtoQx0UQHbkrlEA8YsZ-tW20S4_YgQZkRtN61tzzZ5Es63KH_crZymNi19gwD2kq_9RJu376oqL81YONxJXxRyQawrJCali6YYn7-qqBl9acLDwP0W_jAan7TFNWau1AvRIrP0o3tkBse5NNiaEMvkfxD_5EKtQdKeP6grUe90", - "e": "AQAB", - "kid": "keyID" - } - ] - }`)) + httpmock.RegisterResponder("GET", provider.GetJWKSURL(), + httpmock.NewStringResponder(200, fmt.Sprintf(`{ + "keys": [ + { + "alg": "RS256", + "kty": "RSA", + "use": "sig", + "n": %s, + "e": "AQAB", + "kid": "%s" + } + ] + }`, DummyRSAModulus, jwtKeyID))) // Mock the userinfo endpoint - // TODO: Replace with factory user information // Sub is the Okta user ID, it is not a UUID. tioOfficeOktaUserID := tioOfficeUser.User.LoginGovUUID.String() - httpmock.RegisterResponder("GET", "https://dummy.okta.com/oauth2/default/v1/userinfo", + httpmock.RegisterResponder("GET", provider.GetUserInfoURL(), httpmock.NewStringResponder(200, fmt.Sprintf(`{ "sub": "%s", "name": "name", @@ -871,8 +871,12 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForInvalidUser() { tioOfficeUser := factory.BuildOfficeUserWithRoles(suite.DB(), nil, []roles.RoleType{roles.RoleTypeTIO}) suite.False(tioOfficeUser.Active) + // Build provider + provider, err := factory.BuildOktaProvider(officeProviderName) + suite.NoError(err) + // Mock the necessary Okta endpoints - mockAndActivateOktaEndpoints(tioOfficeUser) + mockAndActivateOktaEndpoints(tioOfficeUser, provider) handlerConfig := suite.HandlerConfig() appnames := handlerConfig.AppNames() @@ -907,11 +911,8 @@ func (suite *AuthSuite) TestRedirectFromLoginGovForInvalidUser() { req = suite.SetupSessionRequest(req, &session, sessionManager) defer goth.ClearProviders() - factoryProvider, err := factory.DummyProviderFactory(officeProviderName) - suite.NoError(err) - goth.UseProviders(factoryProvider) - // TODO: Replace with consts - mockIDToken, err := generateJWTToken("dummyClientID", "https://dummy.okta.com/oauth2/default", stateValue) + goth.UseProviders(provider) + mockIDToken, err := generateJWTToken(provider.GetClientID(), provider.GetIssuerURL(), stateValue) suite.NoError(err) responseBody := fmt.Sprintf(`{"access_token": "mockToken", "id_token": "%s"}`, mockIDToken) // Create the callbackhandler with mock http client for testing diff --git a/pkg/handlers/authentication/okta/provider.go b/pkg/handlers/authentication/okta/provider.go index df135ee92b2..589adfd9cc6 100644 --- a/pkg/handlers/authentication/okta/provider.go +++ b/pkg/handlers/authentication/okta/provider.go @@ -257,6 +257,12 @@ func (op *Provider) GetIssuerURL() string { func (op *Provider) GetLogoutURL() string { return op.orgURL + "/oauth2/v1/logout" } +func (op *Provider) GetJWKSURL() string { + return op.orgURL + "/oauth2/default/.well-known/jwks.json" +} +func (op *Provider) GetOpenIDConfigURL() string { + return op.orgURL + "/oauth2/default/.well-known/openid-configuration" +} // TokenURL returns a full URL to retrieve a user token from okta.mil func (op Provider) TokenURL(r *http.Request) string { From 02942ea02605c2f1b596ae1846f8a2a2aad1f44e Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Wed, 16 Aug 2023 15:34:46 +0000 Subject: [PATCH 21/24] Added pre commit secret key exception for auth handlers test file. --- .pre-commit-config.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ad1e1006753..79b1553b5d5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -50,6 +50,7 @@ repos: pkg/server/testdata/localhost-invalid.key$| pkg/server/testdata/officelocal.key$| pkg/auth/authentication/auth_test.go$| + pkg/handlers/authentication/auth_test.go$| .envrc.local.template| pkg/cli/auth.go$| )$ From eb3a1e77de1df6b5e8e1a2474a2995b3f5f36816 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Wed, 16 Aug 2023 16:07:31 +0000 Subject: [PATCH 22/24] Renamed auth tests and began expansion of okta factory teests --- pkg/factory/okta_factory.go | 20 +++++++++---------- pkg/factory/okta_factory_test.go | 25 ++++++++++++++++++++++++ pkg/handlers/authentication/auth_test.go | 6 +++--- 3 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 pkg/factory/okta_factory_test.go diff --git a/pkg/factory/okta_factory.go b/pkg/factory/okta_factory.go index af614157442..4ed13c6d47f 100644 --- a/pkg/factory/okta_factory.go +++ b/pkg/factory/okta_factory.go @@ -7,12 +7,12 @@ import ( "github.com/transcom/mymove/pkg/handlers/authentication/okta" ) -const dummyOktaOrgURL = "https://dummy.okta.com" -const dummyOktaCallbackURL = "https://dummy.okta.com/auth/callback" -const dummyClientID = "dummyClientID" -const dummySecret = "dummySecret" +const DummyOktaOrgURL = "https://dummy.okta.com" +const DummyOktaCallbackURL = "https://dummy.okta.com/auth/callback" +const DummyClientID = "dummyClientID" +const DummySecret = "dummySecret" -var dummyOIDCScope = []string{"openid", "profile", "email"} +var DummyOIDCScope = []string{"openid", "profile", "email"} type ProviderConfig struct { Name string @@ -39,11 +39,11 @@ func BuildOktaProvider(name string) (*okta.Provider, error) { provider := ProviderConfig{ Name: name, - OrgURL: dummyOktaOrgURL, - CallbackURL: dummyOktaCallbackURL, - ClientID: dummyClientID, - Secret: dummySecret, - Scope: dummyOIDCScope, + OrgURL: DummyOktaOrgURL, + CallbackURL: DummyOktaCallbackURL, + ClientID: DummyClientID, + Secret: DummySecret, + Scope: DummyOIDCScope, Logger: logger, } diff --git a/pkg/factory/okta_factory_test.go b/pkg/factory/okta_factory_test.go new file mode 100644 index 00000000000..1fa530229c7 --- /dev/null +++ b/pkg/factory/okta_factory_test.go @@ -0,0 +1,25 @@ +package factory_test + +import ( + "testing" + + "github.com/stretchr/testify/suite" + "github.com/transcom/mymove/pkg/factory" +) + +type FactorySuite struct { + suite.Suite +} + +func TestFactorySuite(t *testing.T) { + suite.Run(t, new(FactorySuite)) +} + +func (suite *FactorySuite) TestBuildOktaProvider() { + name := "TestProvider" + provider, err := factory.BuildOktaProvider(name) + + suite.NoError(err) + suite.Equal(factory.DummyOktaOrgURL, provider.GetOrgURL()) + suite.Equal(factory.DummyClientID, provider.GetClientID()) +} diff --git a/pkg/handlers/authentication/auth_test.go b/pkg/handlers/authentication/auth_test.go index a1d7cd34626..4025fb5247f 100644 --- a/pkg/handlers/authentication/auth_test.go +++ b/pkg/handlers/authentication/auth_test.go @@ -652,7 +652,7 @@ func (suite *AuthSuite) TestAuthorizeDeactivateOfficeUser() { suite.Equal(authorizationResultUnauthorized, result, "authorizer did not recognize deactivated office user") } -func (suite *AuthSuite) TestRedirectLoginGovErrorMsg() { +func (suite *AuthSuite) TestRedirectOktaErrorMsg() { officeUserID := uuid.Must(uuid.NewV4()) loginGovUUID, _ := uuid.FromString("2400c3c5-019d-4031-9c27-8a553e022297") @@ -734,7 +734,7 @@ func (suite *AuthSuite) TestRedirectLoginGovErrorMsg() { } // Test to make sure the full auth flow works, although we are using mock Okta endpoints -func (suite *AuthSuite) TestRedirectFromLoginGovForValidUser() { +func (suite *AuthSuite) TestRedirectFromOktaForValidUser() { // build a real office user tioOfficeUser := factory.BuildOfficeUserWithRoles(suite.DB(), factory.GetTraitActiveOfficeUser(), @@ -867,7 +867,7 @@ func mockAndActivateOktaEndpoints(tioOfficeUser models.OfficeUser, provider *okt } // Test to make sure the full auth flow works, although we are using mock Okta endpoints -func (suite *AuthSuite) TestRedirectFromLoginGovForInvalidUser() { +func (suite *AuthSuite) TestRedirectFromOktaForInvalidUser() { tioOfficeUser := factory.BuildOfficeUserWithRoles(suite.DB(), nil, []roles.RoleType{roles.RoleTypeTIO}) suite.False(tioOfficeUser.Active) From 16869d6d67ea8cd0cfe9caceb663db2a87031aac Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Wed, 16 Aug 2023 16:15:30 +0000 Subject: [PATCH 23/24] factory --- pkg/factory/okta_factory_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/factory/okta_factory_test.go b/pkg/factory/okta_factory_test.go index 1fa530229c7..e77ed355acc 100644 --- a/pkg/factory/okta_factory_test.go +++ b/pkg/factory/okta_factory_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/suite" + "github.com/transcom/mymove/pkg/factory" ) From 5c4a0c6284f9a85696d4b0aeef1a46ebb5d0f247 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Wed, 16 Aug 2023 16:46:16 +0000 Subject: [PATCH 24/24] json pass string parameter issue with go fix --- pkg/handlers/authentication/auth_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/handlers/authentication/auth_test.go b/pkg/handlers/authentication/auth_test.go index 4025fb5247f..c798d11b154 100644 --- a/pkg/handlers/authentication/auth_test.go +++ b/pkg/handlers/authentication/auth_test.go @@ -833,20 +833,22 @@ func generateJWTToken(aud, iss, nonce string) (string, error) { func mockAndActivateOktaEndpoints(tioOfficeUser models.OfficeUser, provider *okta.Provider) { // Mock the OIDC .well-known openid-configuration endpoint jwksURL := provider.GetJWKSURL() - httpmock.RegisterResponder("GET", provider.GetOpenIDConfigURL(), + openIDConfigURL := provider.GetOpenIDConfigURL() + userInfoURL := provider.GetUserInfoURL() + httpmock.RegisterResponder("GET", openIDConfigURL, httpmock.NewStringResponder(200, fmt.Sprintf(`{ - "jwks_uri": %s + "jwks_uri": "%s" }`, jwksURL))) // Mock the JWKS endpoint to receive keys for JWT verification - httpmock.RegisterResponder("GET", provider.GetJWKSURL(), + httpmock.RegisterResponder("GET", jwksURL, httpmock.NewStringResponder(200, fmt.Sprintf(`{ "keys": [ { "alg": "RS256", "kty": "RSA", "use": "sig", - "n": %s, + "n": "%s", "e": "AQAB", "kid": "%s" } @@ -856,7 +858,7 @@ func mockAndActivateOktaEndpoints(tioOfficeUser models.OfficeUser, provider *okt // Mock the userinfo endpoint // Sub is the Okta user ID, it is not a UUID. tioOfficeOktaUserID := tioOfficeUser.User.LoginGovUUID.String() - httpmock.RegisterResponder("GET", provider.GetUserInfoURL(), + httpmock.RegisterResponder("GET", userInfoURL, httpmock.NewStringResponder(200, fmt.Sprintf(`{ "sub": "%s", "name": "name",