From 99c053d32145956257722d804e1a35e7a384ed7e Mon Sep 17 00:00:00 2001 From: Bryon Nevis Date: Wed, 16 Jun 2021 13:13:22 -0700 Subject: [PATCH] feat(security): Deprecate oauth2 auth method Deprecates oauth2 proxy auth method in favor of stronger JWT-based auth method by removing ability for secrets-config utility to create OAuth2 users. security-proxy-setup accidentally broke OAuth2 functionality by installing a global JWT auth handler intended to secure admin API (can't have two global auth handlers because both auth methods must pass, which is impossible.) Comment rather than remove code in case it needs to be added back in Jakarta stabilization release, otherwise will be removed permanently in both secrets-config and proxy-setup. Closes #3564 Signed-off-by: Bryon Nevis --- .../config/command/proxy/adduser/command.go | 18 ++- .../command/proxy/adduser/command_test.go | 143 +++++++++--------- .../security/config/command/proxy/command.go | 6 +- 3 files changed, 87 insertions(+), 80 deletions(-) diff --git a/internal/security/config/command/proxy/adduser/command.go b/internal/security/config/command/proxy/adduser/command.go index b1f8cebf25..175838e134 100644 --- a/internal/security/config/command/proxy/adduser/command.go +++ b/internal/security/config/command/proxy/adduser/command.go @@ -67,9 +67,10 @@ func NewCommand( flagSet := flag.NewFlagSet(CommandName, flag.ContinueOnError) flagSet.StringVar(&dummy, "confdir", "", "") // handled by bootstrap; duplicated here to prevent arg parsing errors - flagSet.StringVar(&cmd.tokenType, "token-type", "", "Type of token to create: jwt or oauth2") + //flagSet.StringVar(&cmd.tokenType, "token-type", "", "Type of token to create: jwt or oauth2") // #3564: Deprecate for Ireland; might bring back in Jakarta + flagSet.StringVar(&cmd.tokenType, "token-type", "jwt", "Type of token to create: jwt") flagSet.StringVar(&cmd.username, "user", "", "Username of the user to add") - flagSet.StringVar(&cmd.group, "group", "gateway", "Group to which the user belongs, defaults to 'gateway'") + flagSet.StringVar(&cmd.group, "group", "gateway-group", "Group to which the user belongs, defaults to 'gateway-group'") flagSet.StringVar(&cmd.algorithm, "algorithm", "", "Algorithm used for signing the JWT, RS256 or ES256") flagSet.StringVar(&cmd.publicKeyPath, "public_key", "", "Public key (in PEM format) used to validate the JWT.") @@ -87,8 +88,12 @@ func NewCommand( if cmd.tokenType == "" { return nil, fmt.Errorf("%s proxy adduser: argument --token-type is required", os.Args[0]) } - if cmd.tokenType != interfaces.JwtTokenType && cmd.tokenType != interfaces.OAuth2TokenType { - return nil, fmt.Errorf("%s proxy adduser: argument --token-type must be either 'jwt' or 'oauth2'", os.Args[0]) + // #3564: Deprecate for Ireland; commenting in case user community needs back in Jakarta stabilization release + //if cmd.tokenType != interfaces.JwtTokenType && cmd.tokenType != interfaces.OAuth2TokenType { + // return nil, fmt.Errorf("%s proxy adduser: argument --token-type must be either 'jwt' or 'oauth2'", os.Args[0]) + //} + if cmd.tokenType != interfaces.JwtTokenType { + return nil, fmt.Errorf("%s proxy adduser: argument --token-type must be 'jwt'", os.Args[0]) } if cmd.username == "" { return nil, fmt.Errorf("%s proxy adduser: argument --user is required", os.Args[0]) @@ -118,8 +123,9 @@ func (c *cmd) Execute() (statusCode int, err error) { switch c.tokenType { case interfaces.JwtTokenType: statusCode, err = c.ExecuteAddJwt() - case interfaces.OAuth2TokenType: - statusCode, err = c.ExecuteAddOAuth2() + // #3564: Deprecate for Ireland; commenting in case user community needs back in Jakarta stabilization release + //case interfaces.OAuth2TokenType: + // statusCode, err = c.ExecuteAddOAuth2() default: statusCode = interfaces.StatusCodeExitWithError err = fmt.Errorf("unsupported token type %s", c.tokenType) diff --git a/internal/security/config/command/proxy/adduser/command_test.go b/internal/security/config/command/proxy/adduser/command_test.go index 2f7ef11da1..15822200cb 100644 --- a/internal/security/config/command/proxy/adduser/command_test.go +++ b/internal/security/config/command/proxy/adduser/command_test.go @@ -124,75 +124,76 @@ func TestAddUserJWT(t *testing.T) { require.Equal(t, interfaces.StatusCodeExitNormal, code) } +// #3564: Deprecate for Ireland; commenting in case user community needs back in Jakarta stabilization release // TestAddUserOAuth2 tests functionality of adduser command using OAuth2 option -func TestAddUserOAuth2(t *testing.T) { - - // Create a mock logger and grab the default configuration struct - lc := logger.MockLogger{} - config := &config.ConfigurationStruct{} - - // Test variables - tokenType := "oauth2" - jwt := "s0meJWT" - user := "someuser" - redirectUris := "https://placeholder" - - // (placeholder) - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - - // Check to make sure the JWT authorization header exists - assert.NotNil(t, r.Header.Values(internal.AuthHeaderTitle)) - require.Equal(t, internal.BearerLabel+jwt, r.Header.Values(internal.AuthHeaderTitle)[0]) - - switch r.URL.EscapedPath() { - - // Testing --> add a consumer - case "/admin/consumers": - require.Equal(t, "POST", r.Method) - w.WriteHeader(http.StatusCreated) - - // Testing --> enable ACL plugin for specific consumer - case "/admin/consumers/someuser/acls": - require.Equal(t, "POST", r.Method) - w.WriteHeader(http.StatusCreated) - - // Testing --> enable JWT plugin for specific consumer - case "/admin/consumers/someuser/oauth2": - require.Equal(t, "POST", r.Method) - w.WriteHeader(http.StatusCreated) - jsonResponse := map[string]interface{}{ - "key": "bad060a9-0e2b-47ba-98d5-9d622e2322b5", - "client_id": "7240fdd9-1665-419b-a8c5-5691ca03af7c", - "client_secret": "d3191db3-8468-4a3c-87fb-df4fccfee983", - } - json.NewEncoder(w).Encode(jsonResponse) - - // Testing --> fail if we don't recognize the URL in the request - default: - t.Fatal(fmt.Sprintf("Unexpected call to URL %s", r.URL.EscapedPath())) - } - })) - defer ts.Close() - tsURL, err := url.Parse(ts.URL) - require.NoError(t, err) - - config.KongURL.Server = tsURL.Hostname() - config.KongURL.ApplicationPort, _ = strconv.Atoi(tsURL.Port()) - - // Setup command "addUser w/OAuth2" - command, err := NewCommand(lc, config, []string{ - "--token-type", tokenType, - "--user", user, - "--redirect_uris", redirectUris, - "--jwt", jwt, - }) - - require.NoError(t, err) - - // Execute command "addUser w/JWT" - code, err := command.Execute() - - // Assert execution return - require.NoError(t, err) - require.Equal(t, interfaces.StatusCodeExitNormal, code) -} +// func TestAddUserOAuth2(t *testing.T) { + +// // Create a mock logger and grab the default configuration struct +// lc := logger.MockLogger{} +// config := &config.ConfigurationStruct{} + +// // Test variables +// tokenType := "oauth2" +// jwt := "s0meJWT" +// user := "someuser" +// redirectUris := "https://placeholder" + +// // (placeholder) +// ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + +// // Check to make sure the JWT authorization header exists +// assert.NotNil(t, r.Header.Values(internal.AuthHeaderTitle)) +// require.Equal(t, internal.BearerLabel+jwt, r.Header.Values(internal.AuthHeaderTitle)[0]) + +// switch r.URL.EscapedPath() { + +// // Testing --> add a consumer +// case "/admin/consumers": +// require.Equal(t, "POST", r.Method) +// w.WriteHeader(http.StatusCreated) + +// // Testing --> enable ACL plugin for specific consumer +// case "/admin/consumers/someuser/acls": +// require.Equal(t, "POST", r.Method) +// w.WriteHeader(http.StatusCreated) + +// // Testing --> enable JWT plugin for specific consumer +// case "/admin/consumers/someuser/oauth2": +// require.Equal(t, "POST", r.Method) +// w.WriteHeader(http.StatusCreated) +// jsonResponse := map[string]interface{}{ +// "key": "bad060a9-0e2b-47ba-98d5-9d622e2322b5", +// "client_id": "7240fdd9-1665-419b-a8c5-5691ca03af7c", +// "client_secret": "d3191db3-8468-4a3c-87fb-df4fccfee983", +// } +// json.NewEncoder(w).Encode(jsonResponse) + +// // Testing --> fail if we don't recognize the URL in the request +// default: +// t.Fatal(fmt.Sprintf("Unexpected call to URL %s", r.URL.EscapedPath())) +// } +// })) +// defer ts.Close() +// tsURL, err := url.Parse(ts.URL) +// require.NoError(t, err) + +// config.KongURL.Server = tsURL.Hostname() +// config.KongURL.ApplicationPort, _ = strconv.Atoi(tsURL.Port()) + +// // Setup command "addUser w/OAuth2" +// command, err := NewCommand(lc, config, []string{ +// "--token-type", tokenType, +// "--user", user, +// "--redirect_uris", redirectUris, +// "--jwt", jwt, +// }) + +// require.NoError(t, err) + +// // Execute command "addUser w/JWT" +// code, err := command.Execute() + +// // Assert execution return +// require.NoError(t, err) +// require.Equal(t, interfaces.StatusCodeExitNormal, code) +// } diff --git a/internal/security/config/command/proxy/command.go b/internal/security/config/command/proxy/command.go index 7e98a82989..98e5a0e2ef 100644 --- a/internal/security/config/command/proxy/command.go +++ b/internal/security/config/command/proxy/command.go @@ -12,7 +12,6 @@ import ( "github.com/edgexfoundry/edgex-go/internal/security/config/command/proxy/adduser" "github.com/edgexfoundry/edgex-go/internal/security/config/command/proxy/deluser" "github.com/edgexfoundry/edgex-go/internal/security/config/command/proxy/jwt" - "github.com/edgexfoundry/edgex-go/internal/security/config/command/proxy/oauth2" "github.com/edgexfoundry/edgex-go/internal/security/config/command/proxy/tls" "github.com/edgexfoundry/edgex-go/internal/security/config/interfaces" "github.com/edgexfoundry/edgex-go/internal/security/proxy/config" @@ -47,8 +46,9 @@ func NewCommand( command, err = deluser.NewCommand(lc, configuration, args[1:]) case jwt.CommandName: command, err = jwt.NewCommand(lc, configuration, args[1:]) - case oauth2.CommandName: - command, err = oauth2.NewCommand(lc, configuration, args[1:]) + // #3564: Deprecate for Ireland; commenting in case user community needs back in Jakarta stabilization release + //case oauth2.CommandName: + // command, err = oauth2.NewCommand(lc, configuration, args[1:]) default: command = nil err = fmt.Errorf("unsupported command %s", commandName)