From b842a8019aa424d0df6562f942e8af627b4f7032 Mon Sep 17 00:00:00 2001 From: gloriacai01 Date: Wed, 18 Dec 2024 12:07:04 -0500 Subject: [PATCH 1/7] add updateoauthapp cli --- cli/app.go | 90 +++++++++++++++++- cli/client.go | 126 +++++++++++++++++++++++++ cli/client_test.go | 28 ++++++ testutils/inject/app_service_client.go | 12 +++ 4 files changed, 251 insertions(+), 5 deletions(-) diff --git a/cli/app.go b/cli/app.go index ba15a22ca00..b1047cb729f 100644 --- a/cli/app.go +++ b/cli/app.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/urfave/cli/v2" + apppb "go.viam.com/api/app/v1" ) // CLI flags. @@ -124,11 +125,18 @@ const ( packageMetadataFlagFramework = "model_framework" - authApplicationFlagName = "application-name" - authApplicationFlagApplicationID = "application-id" - authApplicationFlagOriginURIs = "origin-uris" - authApplicationFlagRedirectURIs = "redirect-uris" - authApplicationFlagLogoutURI = "logout-uri" + authApplicationFlagName = "application-name" + authApplicationFlagApplicationID = "application-id" + authApplicationFlagOriginURIs = "origin-uris" + authApplicationFlagRedirectURIs = "redirect-uris" + authApplicationFlagLogoutURI = "logout-uri" + authApplicationFlagClientID = "client-id" + authApplicationFlagClientName = "client-name" + authApplicationFlagClientAuthentication = "client-authentication" + authApplicationFlagPKCE = "pkce" + authApplicationFlagEnabledGrants = "enabled-grants" + authApplicationFlagURLValidation = "url-validation" + unspecified = "unspecified" cpFlagRecursive = "recursive" cpFlagPreserve = "preserve" @@ -424,6 +432,78 @@ var app = &cli.App{ Usage: "work with organizations", HideHelpCommand: true, Subcommands: []*cli.Command{ + { + Name: "auth-service", + Usage: "manage the oauth applications for an organization", + UsageText: createUsageText("oauth applications", []string{generalFlagOrgID}, true), + Subcommands: []*cli.Command{ + { + Name: "update", + Usage: "update an oauth application", + UsageText: createUsageText("update", []string{ + generalFlagOrgID, + authApplicationFlagClientID, authApplicationFlagClientName, authApplicationFlagClientAuthentication, authApplicationFlagURLValidation, authApplicationFlagPKCE, authApplicationFlagOriginURIs, + authApplicationFlagRedirectURIs, authApplicationFlagLogoutURI, authApplicationFlagEnabledGrants, + }, false), + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: generalFlagOrgID, + Required: true, + Usage: "organization ID that will be tied to auth application", + }, + &cli.StringFlag{ + Name: authApplicationFlagClientID, + Usage: "id for the oauth application", + Required: true, + }, + &cli.StringFlag{ + Name: authApplicationFlagClientName, + Usage: "updated name for the oauth application", + Required: true, + }, + &cli.StringFlag{ + Name: authApplicationFlagClientAuthentication, + Usage: "updated client authentication policy for the oauth application. can be one of " + allEnumValues(clientAuthenticationPrefix, apppb.ClientAuthentication_value), + Required: false, + Value: unspecified, + }, + &cli.StringFlag{ + Name: authApplicationFlagURLValidation, + Usage: "updated url validation for the oauth application. can be one of " + allEnumValues(urlValidationPrefix, apppb.URLValidation_value), + Required: false, + Value: unspecified, + }, + &cli.StringFlag{ + Name: authApplicationFlagPKCE, + Usage: "updated pkce for the oauth application. can be one of " + allEnumValues(pkcePrefix, apppb.PKCE_value), + Required: false, + Value: unspecified, + }, + &cli.StringSliceFlag{ + Name: authApplicationFlagOriginURIs, + Usage: "updated comma separated origin uris for the oauth application", + Required: false, + }, + &cli.StringSliceFlag{ + Name: authApplicationFlagRedirectURIs, + Usage: "updated comma separated redirect uris for the oauth application", + Required: false, + }, + &cli.StringFlag{ + Name: authApplicationFlagLogoutURI, + Usage: "updated logout uri for the oauth application", + Required: false, + }, + &cli.StringSliceFlag{ + Name: authApplicationFlagEnabledGrants, + Usage: "updated comma separated enabled grants for the auth application. values can be of " + allEnumValues(enabledGrantPrefix, apppb.EnabledGrant_value), + Required: false, + }, + }, + Action: createCommandWithT[updateOAuthAppArgs](UpdateOAuthAppAction), + }, + }, + }, { Name: "list", Usage: "list organizations for the current user", diff --git a/cli/client.go b/cli/client.go index 6ffc908bddc..50cd1c23866 100644 --- a/cli/client.go +++ b/cli/client.go @@ -14,6 +14,7 @@ import ( "os/signal" "path/filepath" "runtime/debug" + "slices" "strings" "time" @@ -2050,3 +2051,128 @@ func logEntryFieldsToString(fields []*structpb.Struct) (string, error) { } return message + "}", nil } + +type updateOAuthAppArgs struct { + OrgID string + ClientID string + ClientName string + ClientAuthentication string + Pkce string + LogoutURI string + UrlValidation string + OriginURIs []string + RedirectURIs []string + EnabledGrants []string +} + +const ( + clientAuthenticationPrefix = "CLIENT_AUTHENTICATION_" + pkcePrefix = "PKCE_" + urlValidationPrefix = "URL_VALIDATION_" + enabledGrantPrefix = "ENABLED_GRANT_" +) + +func allEnumValues(prefixToTrim string, enumValueMap map[string]int32) string { + var formattedValues []string + for values := range enumValueMap { + formattedValue := strings.ToLower(strings.TrimPrefix(values, prefixToTrim)) + formattedValues = append(formattedValues, formattedValue) + } + slices.Sort(formattedValues) + return "[" + strings.Join(formattedValues, ", ") + "]" +} + +// UpdateAuthApplicationAction is the corresponding action for 'auth-app update'. +func UpdateOAuthAppAction(c *cli.Context, args updateOAuthAppArgs) error { + client, err := newViamClient(c) + if err != nil { + return err + } + + return client.updateOAuthAppAction(c, args) +} + +func (c *viamClient) updateOAuthAppAction(cCtx *cli.Context, args updateOAuthAppArgs) error { + if err := c.ensureLoggedIn(); err != nil { + return err + } + + req, err := createUpdateOAuthAppRequest(args) + if err != nil { + return err + } + + _, err = c.client.UpdateOAuthApp(c.c.Context, req) + if err != nil { + return err + } + + infof(cCtx.App.Writer, "Successfully updated oauth application") + return nil +} + +func createUpdateOAuthAppRequest(args updateOAuthAppArgs) (*apppb.UpdateOAuthAppRequest, error) { + orgID := args.OrgID + clientID := args.ClientID + clientName := args.ClientName + clientAuthentication := args.ClientAuthentication + urlValidation := args.UrlValidation + pkce := args.Pkce + originURIs := args.OriginURIs + redirectURIs := args.RedirectURIs + logoutURI := args.LogoutURI + enabledGrants := args.EnabledGrants + + clientAuthenticationEnum, ok := apppb.ClientAuthentication_value[clientAuthenticationPrefix+strings.ToUpper(clientAuthentication)] + if !ok { + return nil, errors.Errorf("%s must be a valid ClientAuthentication, got %s. See `viam organizations auth-service update --help` for supported options", + authApplicationFlagClientAuthentication, clientAuthentication) + } + pkceEnum, ok := apppb.PKCE_value[pkcePrefix+strings.ToUpper(pkce)] + if !ok { + return nil, errors.Errorf("%s must be a valid PKCE, got %s. See `viam organizations auth-service update --help` for supported options", + authApplicationFlagPKCE, pkce) + } + urlValidationEnum, ok := apppb.URLValidation_value[urlValidationPrefix+strings.ToUpper(urlValidation)] + if !ok { + return nil, errors.Errorf("%s must be a valid UrlValidation, got %s. See `viam organizations auth-service update --help` for supported options", + authApplicationFlagURLValidation, urlValidation) + } + + egProto, err := enabledGrantsToProto(enabledGrants) + if err != nil { + return nil, err + } + + req := &apppb.UpdateOAuthAppRequest{ + OrgId: orgID, + ClientId: clientID, + ClientName: clientName, + OauthConfig: &apppb.OAuthConfig{ + ClientAuthentication: apppb.ClientAuthentication(clientAuthenticationEnum), + Pkce: apppb.PKCE(pkceEnum), + UrlValidation: apppb.URLValidation(urlValidationEnum), + OriginUris: originURIs, + RedirectUris: redirectURIs, + LogoutUri: logoutURI, + EnabledGrants: egProto, + }, + } + return req, nil +} + +func enabledGrantsToProto(enabledGrants []string) ([]apppb.EnabledGrant, error) { + if enabledGrants == nil { + return nil, nil + } + enabledGrantsProto := make([]apppb.EnabledGrant, len(enabledGrants)) + for i, eg := range enabledGrants { + enum, ok := apppb.EnabledGrant_value[enabledGrantPrefix+strings.ToUpper(eg)] + if !ok { + return nil, errors.Errorf("%s must consist of valid EnabledGrants, got %s. See `viam organizations auth-service update --help` for supported options", + authApplicationFlagEnabledGrants, eg) + } + enabledGrantsProto[i] = apppb.EnabledGrant(enum) + } + return enabledGrantsProto, nil +} diff --git a/cli/client_test.go b/cli/client_test.go index bda3d9e26ca..1eb40a488b9 100644 --- a/cli/client_test.go +++ b/cli/client_test.go @@ -1001,3 +1001,31 @@ func TestShellFileCopy(t *testing.T) { }) }) } + +func TestUpdateOAuthAppAction(t *testing.T) { + updateOAuthAppFunc := func(ctx context.Context, in *apppb.UpdateOAuthAppRequest, + opts ...grpc.CallOption, + ) (*apppb.UpdateOAuthAppResponse, error) { + return &apppb.UpdateOAuthAppResponse{}, nil + } + asc := &inject.AppServiceClient{ + UpdateOAuthAppFunc: updateOAuthAppFunc, + } + + flags := make(map[string]any) + flags[generalFlagOrgID] = "org-id" + flags[authApplicationFlagClientID] = "client-id" + flags[authApplicationFlagClientName] = "client-name" + flags[authApplicationFlagClientAuthentication] = "required" + flags[authApplicationFlagURLValidation] = "allow_wildcards" + flags[authApplicationFlagPKCE] = "not_required" + flags[authApplicationFlagOriginURIs] = []string{"https://woof.com/login", "https://arf.com/"} + flags[authApplicationFlagRedirectURIs] = []string{"https://woof.com/home", "https://arf.com/home"} + flags[authApplicationFlagLogoutURI] = "https://woof.com/logout" + flags[authApplicationFlagEnabledGrants] = []string{"implicit", "password"} + cCtx, ac, out, errOut := setup(asc, nil, nil, nil, flags, "token") + + test.That(t, ac.updateOAuthAppAction(cCtx, parseStructFromCtx[updateOAuthAppArgs](cCtx)), test.ShouldBeNil) + test.That(t, len(errOut.messages), test.ShouldEqual, 0) + test.That(t, out.messages[1], test.ShouldContainSubstring, "Successfully updated oauth application") +} diff --git a/testutils/inject/app_service_client.go b/testutils/inject/app_service_client.go index 45f35604723..74a129d134f 100644 --- a/testutils/inject/app_service_client.go +++ b/testutils/inject/app_service_client.go @@ -56,6 +56,8 @@ type AppServiceClient struct { opts ...grpc.CallOption) (*apppb.OrganizationSetLogoResponse, error) OrganizationGetLogoFunc func(ctx context.Context, in *apppb.OrganizationGetLogoRequest, opts ...grpc.CallOption) (*apppb.OrganizationGetLogoResponse, error) + UpdateOAuthAppFunc func(ctx context.Context, in *apppb.UpdateOAuthAppRequest, + opts ...grpc.CallOption) (*apppb.UpdateOAuthAppResponse, error) CreateLocationFunc func(ctx context.Context, in *apppb.CreateLocationRequest, opts ...grpc.CallOption) (*apppb.CreateLocationResponse, error) GetLocationFunc func(ctx context.Context, in *apppb.GetLocationRequest, @@ -403,6 +405,16 @@ func (asc *AppServiceClient) OrganizationGetLogo( return asc.OrganizationGetLogoFunc(ctx, in, opts...) } +// UpdateOAuthApp calls the injected UpdateOAuthAppFunc or the real version. +func (asc *AppServiceClient) UpdateOAuthApp( + ctx context.Context, in *apppb.UpdateOAuthAppRequest, opts ...grpc.CallOption, +) (*apppb.UpdateOAuthAppResponse, error) { + if asc.UpdateOAuthAppFunc == nil { + return asc.AppServiceClient.UpdateOAuthApp(ctx, in, opts...) + } + return asc.UpdateOAuthAppFunc(ctx, in, opts...) +} + // CreateLocation calls the injected CreateLocationFunc or the real version. func (asc *AppServiceClient) CreateLocation( ctx context.Context, in *apppb.CreateLocationRequest, opts ...grpc.CallOption, From 5ff53536bf768baa5b4bde34b58010d608f3846a Mon Sep 17 00:00:00 2001 From: gloriacai01 Date: Wed, 18 Dec 2024 12:17:49 -0500 Subject: [PATCH 2/7] add comments --- cli/app.go | 4 ++-- cli/client.go | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cli/app.go b/cli/app.go index b1047cb729f..10ff391cbb8 100644 --- a/cli/app.go +++ b/cli/app.go @@ -449,11 +449,11 @@ var app = &cli.App{ &cli.StringFlag{ Name: generalFlagOrgID, Required: true, - Usage: "organization ID that will be tied to auth application", + Usage: "organization ID that will be tied to oauth application", }, &cli.StringFlag{ Name: authApplicationFlagClientID, - Usage: "id for the oauth application", + Usage: "id for the oauth application to be updated", Required: true, }, &cli.StringFlag{ diff --git a/cli/client.go b/cli/client.go index 50cd1c23866..4fe4b2b2b76 100644 --- a/cli/client.go +++ b/cli/client.go @@ -2072,6 +2072,7 @@ const ( enabledGrantPrefix = "ENABLED_GRANT_" ) +// allEnumValues returns the possible values we accept for a given proto enum func allEnumValues(prefixToTrim string, enumValueMap map[string]int32) string { var formattedValues []string for values := range enumValueMap { @@ -2082,7 +2083,7 @@ func allEnumValues(prefixToTrim string, enumValueMap map[string]int32) string { return "[" + strings.Join(formattedValues, ", ") + "]" } -// UpdateAuthApplicationAction is the corresponding action for 'auth-app update'. +// UpdateOAuthAppAction is the corresponding action for 'auth-service update'. func UpdateOAuthAppAction(c *cli.Context, args updateOAuthAppArgs) error { client, err := newViamClient(c) if err != nil { From a7cb4736a766966d88aa70e5e207f351a7cb9dd6 Mon Sep 17 00:00:00 2001 From: gloriacai01 Date: Wed, 18 Dec 2024 17:03:00 -0500 Subject: [PATCH 3/7] lint --- cli/app.go | 23 ++++++++++++++--------- cli/client.go | 16 ++++++++++------ 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/cli/app.go b/cli/app.go index 10ff391cbb8..b4c0c275506 100644 --- a/cli/app.go +++ b/cli/app.go @@ -442,7 +442,8 @@ var app = &cli.App{ Usage: "update an oauth application", UsageText: createUsageText("update", []string{ generalFlagOrgID, - authApplicationFlagClientID, authApplicationFlagClientName, authApplicationFlagClientAuthentication, authApplicationFlagURLValidation, authApplicationFlagPKCE, authApplicationFlagOriginURIs, + authApplicationFlagClientID, authApplicationFlagClientName, authApplicationFlagClientAuthentication, + authApplicationFlagURLValidation, authApplicationFlagPKCE, authApplicationFlagOriginURIs, authApplicationFlagRedirectURIs, authApplicationFlagLogoutURI, authApplicationFlagEnabledGrants, }, false), Flags: []cli.Flag{ @@ -462,20 +463,23 @@ var app = &cli.App{ Required: true, }, &cli.StringFlag{ - Name: authApplicationFlagClientAuthentication, - Usage: "updated client authentication policy for the oauth application. can be one of " + allEnumValues(clientAuthenticationPrefix, apppb.ClientAuthentication_value), + Name: authApplicationFlagClientAuthentication, + Usage: "updated client authentication policy for the oauth application. can be one of " + + allEnumValues(clientAuthenticationPrefix, apppb.ClientAuthentication_value), Required: false, Value: unspecified, }, &cli.StringFlag{ - Name: authApplicationFlagURLValidation, - Usage: "updated url validation for the oauth application. can be one of " + allEnumValues(urlValidationPrefix, apppb.URLValidation_value), + Name: authApplicationFlagURLValidation, + Usage: "updated url validation for the oauth application. can be one of " + + allEnumValues(urlValidationPrefix, apppb.URLValidation_value), Required: false, Value: unspecified, }, &cli.StringFlag{ - Name: authApplicationFlagPKCE, - Usage: "updated pkce for the oauth application. can be one of " + allEnumValues(pkcePrefix, apppb.PKCE_value), + Name: authApplicationFlagPKCE, + Usage: "updated pkce for the oauth application. can be one of " + + allEnumValues(pkcePrefix, apppb.PKCE_value), Required: false, Value: unspecified, }, @@ -495,8 +499,9 @@ var app = &cli.App{ Required: false, }, &cli.StringSliceFlag{ - Name: authApplicationFlagEnabledGrants, - Usage: "updated comma separated enabled grants for the auth application. values can be of " + allEnumValues(enabledGrantPrefix, apppb.EnabledGrant_value), + Name: authApplicationFlagEnabledGrants, + Usage: "updated comma separated enabled grants for the auth application. values can be of " + + allEnumValues(enabledGrantPrefix, apppb.EnabledGrant_value), Required: false, }, }, diff --git a/cli/client.go b/cli/client.go index 4fe4b2b2b76..fc5154c7fff 100644 --- a/cli/client.go +++ b/cli/client.go @@ -2059,7 +2059,7 @@ type updateOAuthAppArgs struct { ClientAuthentication string Pkce string LogoutURI string - UrlValidation string + UrlValidation string //nolint:revive,stylecheck OriginURIs []string RedirectURIs []string EnabledGrants []string @@ -2072,7 +2072,7 @@ const ( enabledGrantPrefix = "ENABLED_GRANT_" ) -// allEnumValues returns the possible values we accept for a given proto enum +// allEnumValues returns the possible values we accept for a given proto enum. func allEnumValues(prefixToTrim string, enumValueMap map[string]int32) string { var formattedValues []string for values := range enumValueMap { @@ -2126,17 +2126,20 @@ func createUpdateOAuthAppRequest(args updateOAuthAppArgs) (*apppb.UpdateOAuthApp clientAuthenticationEnum, ok := apppb.ClientAuthentication_value[clientAuthenticationPrefix+strings.ToUpper(clientAuthentication)] if !ok { - return nil, errors.Errorf("%s must be a valid ClientAuthentication, got %s. See `viam organizations auth-service update --help` for supported options", + return nil, errors.Errorf("%s must be a valid ClientAuthentication, got %s. "+ + "See `viam organizations auth-service update --help` for supported options", authApplicationFlagClientAuthentication, clientAuthentication) } pkceEnum, ok := apppb.PKCE_value[pkcePrefix+strings.ToUpper(pkce)] if !ok { - return nil, errors.Errorf("%s must be a valid PKCE, got %s. See `viam organizations auth-service update --help` for supported options", + return nil, errors.Errorf("%s must be a valid PKCE, got %s. "+ + "See `viam organizations auth-service update --help` for supported options", authApplicationFlagPKCE, pkce) } urlValidationEnum, ok := apppb.URLValidation_value[urlValidationPrefix+strings.ToUpper(urlValidation)] if !ok { - return nil, errors.Errorf("%s must be a valid UrlValidation, got %s. See `viam organizations auth-service update --help` for supported options", + return nil, errors.Errorf("%s must be a valid UrlValidation, got %s. "+ + "See `viam organizations auth-service update --help` for supported options", authApplicationFlagURLValidation, urlValidation) } @@ -2170,7 +2173,8 @@ func enabledGrantsToProto(enabledGrants []string) ([]apppb.EnabledGrant, error) for i, eg := range enabledGrants { enum, ok := apppb.EnabledGrant_value[enabledGrantPrefix+strings.ToUpper(eg)] if !ok { - return nil, errors.Errorf("%s must consist of valid EnabledGrants, got %s. See `viam organizations auth-service update --help` for supported options", + return nil, errors.Errorf("%s must consist of valid EnabledGrants, got %s. "+ + "See `viam organizations auth-service update --help` for supported options", authApplicationFlagEnabledGrants, eg) } enabledGrantsProto[i] = apppb.EnabledGrant(enum) From 3b461aa57cb47a667d2f8b9e6fe2883e129fa2b2 Mon Sep 17 00:00:00 2001 From: gloriacai01 Date: Thu, 19 Dec 2024 12:32:02 -0500 Subject: [PATCH 4/7] change wording --- cli/app.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/app.go b/cli/app.go index b4c0c275506..8ce13ddeb62 100644 --- a/cli/app.go +++ b/cli/app.go @@ -435,7 +435,7 @@ var app = &cli.App{ { Name: "auth-service", Usage: "manage the oauth applications for an organization", - UsageText: createUsageText("oauth applications", []string{generalFlagOrgID}, true), + UsageText: createUsageText("organizations auth-service", []string{generalFlagOrgID}, true), Subcommands: []*cli.Command{ { Name: "update", @@ -450,7 +450,7 @@ var app = &cli.App{ &cli.StringFlag{ Name: generalFlagOrgID, Required: true, - Usage: "organization ID that will be tied to oauth application", + Usage: "organization ID that is tied to the oauth application", }, &cli.StringFlag{ Name: authApplicationFlagClientID, From 6680cbdb2c7a183f254cb4d15a8e12094fe688d5 Mon Sep 17 00:00:00 2001 From: gloriacai01 Date: Thu, 19 Dec 2024 12:59:04 -0500 Subject: [PATCH 5/7] add oauth-app command flag --- cli/app.go | 139 +++++++++++++++++++++++++++-------------------------- 1 file changed, 70 insertions(+), 69 deletions(-) diff --git a/cli/app.go b/cli/app.go index 8ce13ddeb62..1e2c87edd60 100644 --- a/cli/app.go +++ b/cli/app.go @@ -136,6 +136,8 @@ const ( authApplicationFlagPKCE = "pkce" authApplicationFlagEnabledGrants = "enabled-grants" authApplicationFlagURLValidation = "url-validation" + authServiceFlag = "auth-service" + authOAuthAppFlag = "oauth-app" unspecified = "unspecified" cpFlagRecursive = "recursive" @@ -433,79 +435,78 @@ var app = &cli.App{ HideHelpCommand: true, Subcommands: []*cli.Command{ { - Name: "auth-service", - Usage: "manage the oauth applications for an organization", - UsageText: createUsageText("organizations auth-service", []string{generalFlagOrgID}, true), + Name: "auth-service", + Usage: "manage auth-service", Subcommands: []*cli.Command{ { - Name: "update", - Usage: "update an oauth application", - UsageText: createUsageText("update", []string{ - generalFlagOrgID, - authApplicationFlagClientID, authApplicationFlagClientName, authApplicationFlagClientAuthentication, - authApplicationFlagURLValidation, authApplicationFlagPKCE, authApplicationFlagOriginURIs, - authApplicationFlagRedirectURIs, authApplicationFlagLogoutURI, authApplicationFlagEnabledGrants, - }, false), - Flags: []cli.Flag{ - &cli.StringFlag{ - Name: generalFlagOrgID, - Required: true, - Usage: "organization ID that is tied to the oauth application", - }, - &cli.StringFlag{ - Name: authApplicationFlagClientID, - Usage: "id for the oauth application to be updated", - Required: true, - }, - &cli.StringFlag{ - Name: authApplicationFlagClientName, - Usage: "updated name for the oauth application", - Required: true, - }, - &cli.StringFlag{ - Name: authApplicationFlagClientAuthentication, - Usage: "updated client authentication policy for the oauth application. can be one of " + - allEnumValues(clientAuthenticationPrefix, apppb.ClientAuthentication_value), - Required: false, - Value: unspecified, - }, - &cli.StringFlag{ - Name: authApplicationFlagURLValidation, - Usage: "updated url validation for the oauth application. can be one of " + - allEnumValues(urlValidationPrefix, apppb.URLValidation_value), - Required: false, - Value: unspecified, - }, - &cli.StringFlag{ - Name: authApplicationFlagPKCE, - Usage: "updated pkce for the oauth application. can be one of " + - allEnumValues(pkcePrefix, apppb.PKCE_value), - Required: false, - Value: unspecified, - }, - &cli.StringSliceFlag{ - Name: authApplicationFlagOriginURIs, - Usage: "updated comma separated origin uris for the oauth application", - Required: false, - }, - &cli.StringSliceFlag{ - Name: authApplicationFlagRedirectURIs, - Usage: "updated comma separated redirect uris for the oauth application", - Required: false, - }, - &cli.StringFlag{ - Name: authApplicationFlagLogoutURI, - Usage: "updated logout uri for the oauth application", - Required: false, - }, - &cli.StringSliceFlag{ - Name: authApplicationFlagEnabledGrants, - Usage: "updated comma separated enabled grants for the auth application. values can be of " + - allEnumValues(enabledGrantPrefix, apppb.EnabledGrant_value), - Required: false, + Name: "oauth-app", + Usage: "manage the oauth applications for an organization", + Subcommands: []*cli.Command{ + { + Name: "update", + Usage: "update an oauth application", + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: generalFlagOrgID, + Required: true, + Usage: "organization ID that is tied to the oauth application", + }, + &cli.StringFlag{ + Name: authApplicationFlagClientID, + Usage: "id for the oauth application to be updated", + Required: true, + }, + &cli.StringFlag{ + Name: authApplicationFlagClientName, + Usage: "updated name for the oauth application", + Required: true, + }, + &cli.StringFlag{ + Name: authApplicationFlagClientAuthentication, + Usage: "updated client authentication policy for the oauth application. can be one of " + + allEnumValues(clientAuthenticationPrefix, apppb.ClientAuthentication_value), + Required: false, + Value: unspecified, + }, + &cli.StringFlag{ + Name: authApplicationFlagURLValidation, + Usage: "updated url validation for the oauth application. can be one of " + + allEnumValues(urlValidationPrefix, apppb.URLValidation_value), + Required: false, + Value: unspecified, + }, + &cli.StringFlag{ + Name: authApplicationFlagPKCE, + Usage: "updated pkce for the oauth application. can be one of " + + allEnumValues(pkcePrefix, apppb.PKCE_value), + Required: false, + Value: unspecified, + }, + &cli.StringSliceFlag{ + Name: authApplicationFlagOriginURIs, + Usage: "updated comma separated origin uris for the oauth application", + Required: false, + }, + &cli.StringSliceFlag{ + Name: authApplicationFlagRedirectURIs, + Usage: "updated comma separated redirect uris for the oauth application", + Required: false, + }, + &cli.StringFlag{ + Name: authApplicationFlagLogoutURI, + Usage: "updated logout uri for the oauth application", + Required: false, + }, + &cli.StringSliceFlag{ + Name: authApplicationFlagEnabledGrants, + Usage: "updated comma separated enabled grants for the auth application. values can be of " + + allEnumValues(enabledGrantPrefix, apppb.EnabledGrant_value), + Required: false, + }, + }, + Action: createCommandWithT[updateOAuthAppArgs](UpdateOAuthAppAction), }, }, - Action: createCommandWithT[updateOAuthAppArgs](UpdateOAuthAppAction), }, }, }, From ec0b6ad22d2f2e286337090d9a8fa26f6803d236 Mon Sep 17 00:00:00 2001 From: gloriacai01 Date: Thu, 19 Dec 2024 14:30:30 -0500 Subject: [PATCH 6/7] make client name not required --- cli/app.go | 2 +- cli/client.go | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cli/app.go b/cli/app.go index 1e2c87edd60..b93e3f80c39 100644 --- a/cli/app.go +++ b/cli/app.go @@ -459,7 +459,7 @@ var app = &cli.App{ &cli.StringFlag{ Name: authApplicationFlagClientName, Usage: "updated name for the oauth application", - Required: true, + Required: false, }, &cli.StringFlag{ Name: authApplicationFlagClientAuthentication, diff --git a/cli/client.go b/cli/client.go index fc5154c7fff..370f77ee3d7 100644 --- a/cli/client.go +++ b/cli/client.go @@ -2083,7 +2083,7 @@ func allEnumValues(prefixToTrim string, enumValueMap map[string]int32) string { return "[" + strings.Join(formattedValues, ", ") + "]" } -// UpdateOAuthAppAction is the corresponding action for 'auth-service update'. +// UpdateOAuthAppAction is the corresponding action for 'oauth-app update'. func UpdateOAuthAppAction(c *cli.Context, args updateOAuthAppArgs) error { client, err := newViamClient(c) if err != nil { @@ -2108,7 +2108,7 @@ func (c *viamClient) updateOAuthAppAction(cCtx *cli.Context, args updateOAuthApp return err } - infof(cCtx.App.Writer, "Successfully updated oauth application") + printf(cCtx.App.Writer, "Successfully updated oauth application %s", args.ClientID) return nil } @@ -2127,19 +2127,21 @@ func createUpdateOAuthAppRequest(args updateOAuthAppArgs) (*apppb.UpdateOAuthApp clientAuthenticationEnum, ok := apppb.ClientAuthentication_value[clientAuthenticationPrefix+strings.ToUpper(clientAuthentication)] if !ok { return nil, errors.Errorf("%s must be a valid ClientAuthentication, got %s. "+ - "See `viam organizations auth-service update --help` for supported options", + "See `viam organizations auth-service oauth-app update --help` for supported options", authApplicationFlagClientAuthentication, clientAuthentication) } + pkceEnum, ok := apppb.PKCE_value[pkcePrefix+strings.ToUpper(pkce)] if !ok { return nil, errors.Errorf("%s must be a valid PKCE, got %s. "+ - "See `viam organizations auth-service update --help` for supported options", + "See `viam organizations auth-service oauth-app update --help` for supported options", authApplicationFlagPKCE, pkce) } + urlValidationEnum, ok := apppb.URLValidation_value[urlValidationPrefix+strings.ToUpper(urlValidation)] if !ok { return nil, errors.Errorf("%s must be a valid UrlValidation, got %s. "+ - "See `viam organizations auth-service update --help` for supported options", + "See `viam organizations auth-service oauth-app update --help` for supported options", authApplicationFlagURLValidation, urlValidation) } @@ -2174,7 +2176,7 @@ func enabledGrantsToProto(enabledGrants []string) ([]apppb.EnabledGrant, error) enum, ok := apppb.EnabledGrant_value[enabledGrantPrefix+strings.ToUpper(eg)] if !ok { return nil, errors.Errorf("%s must consist of valid EnabledGrants, got %s. "+ - "See `viam organizations auth-service update --help` for supported options", + "See `viam organizations auth-service oauth-app update --help` for supported options", authApplicationFlagEnabledGrants, eg) } enabledGrantsProto[i] = apppb.EnabledGrant(enum) From b1c63509614373a8d3b3f9b401346eb4a50b2bf9 Mon Sep 17 00:00:00 2001 From: gloriacai01 Date: Fri, 20 Dec 2024 10:49:07 -0500 Subject: [PATCH 7/7] add tests to check invalid enum value error messages --- cli/app.go | 73 ++++++++++++++++++++++++---------------------- cli/client.go | 16 +++++----- cli/client_test.go | 66 +++++++++++++++++++++++++++++++---------- 3 files changed, 96 insertions(+), 59 deletions(-) diff --git a/cli/app.go b/cli/app.go index b93e3f80c39..c211cf0c6cf 100644 --- a/cli/app.go +++ b/cli/app.go @@ -125,20 +125,23 @@ const ( packageMetadataFlagFramework = "model_framework" - authApplicationFlagName = "application-name" - authApplicationFlagApplicationID = "application-id" - authApplicationFlagOriginURIs = "origin-uris" - authApplicationFlagRedirectURIs = "redirect-uris" - authApplicationFlagLogoutURI = "logout-uri" - authApplicationFlagClientID = "client-id" - authApplicationFlagClientName = "client-name" - authApplicationFlagClientAuthentication = "client-authentication" - authApplicationFlagPKCE = "pkce" - authApplicationFlagEnabledGrants = "enabled-grants" - authApplicationFlagURLValidation = "url-validation" - authServiceFlag = "auth-service" - authOAuthAppFlag = "oauth-app" - unspecified = "unspecified" + // TODO: APP-6993 remove these flags. + authApplicationFlagName = "application-name" + authApplicationFlagApplicationID = "application-id" + authApplicationFlagOriginURIs = "origin-uris" + authApplicationFlagRedirectURIs = "redirect-uris" + authApplicationFlagLogoutURI = "logout-uri" + + oauthAppFlagClientID = "client-id" + oauthAppFlagClientName = "client-name" + oauthAppFlagClientAuthentication = "client-authentication" + oauthAppFlagPKCE = "pkce" + oauthAppFlagEnabledGrants = "enabled-grants" + oauthAppFlagURLValidation = "url-validation" + oauthAppFlagOriginURIs = "origin-uris" + oauthAppFlagRedirectURIs = "redirect-uris" + oauthAppFlagLogoutURI = "logout-uri" + unspecified = "unspecified" cpFlagRecursive = "recursive" cpFlagPreserve = "preserve" @@ -440,66 +443,66 @@ var app = &cli.App{ Subcommands: []*cli.Command{ { Name: "oauth-app", - Usage: "manage the oauth applications for an organization", + Usage: "manage the OAuth applications for an organization", Subcommands: []*cli.Command{ { Name: "update", - Usage: "update an oauth application", + Usage: "update an OAuth application", Flags: []cli.Flag{ &cli.StringFlag{ Name: generalFlagOrgID, Required: true, - Usage: "organization ID that is tied to the oauth application", + Usage: "organization ID that is tied to the OAuth application", }, &cli.StringFlag{ - Name: authApplicationFlagClientID, - Usage: "id for the oauth application to be updated", + Name: oauthAppFlagClientID, + Usage: "id for the OAuth application to be updated", Required: true, }, &cli.StringFlag{ - Name: authApplicationFlagClientName, - Usage: "updated name for the oauth application", + Name: oauthAppFlagClientName, + Usage: "updated name for the OAuth application", Required: false, }, &cli.StringFlag{ - Name: authApplicationFlagClientAuthentication, - Usage: "updated client authentication policy for the oauth application. can be one of " + + Name: oauthAppFlagClientAuthentication, + Usage: "updated client authentication policy for the OAuth application. can be one of " + allEnumValues(clientAuthenticationPrefix, apppb.ClientAuthentication_value), Required: false, Value: unspecified, }, &cli.StringFlag{ - Name: authApplicationFlagURLValidation, - Usage: "updated url validation for the oauth application. can be one of " + + Name: oauthAppFlagURLValidation, + Usage: "updated url validation for the OAuth application. can be one of " + allEnumValues(urlValidationPrefix, apppb.URLValidation_value), Required: false, Value: unspecified, }, &cli.StringFlag{ - Name: authApplicationFlagPKCE, - Usage: "updated pkce for the oauth application. can be one of " + + Name: oauthAppFlagPKCE, + Usage: "updated pkce for the OAuth application. can be one of " + allEnumValues(pkcePrefix, apppb.PKCE_value), Required: false, Value: unspecified, }, &cli.StringSliceFlag{ - Name: authApplicationFlagOriginURIs, - Usage: "updated comma separated origin uris for the oauth application", + Name: oauthAppFlagOriginURIs, + Usage: "updated comma separated origin uris for the OAuth application", Required: false, }, &cli.StringSliceFlag{ - Name: authApplicationFlagRedirectURIs, - Usage: "updated comma separated redirect uris for the oauth application", + Name: oauthAppFlagRedirectURIs, + Usage: "updated comma separated redirect uris for the OAuth application", Required: false, }, &cli.StringFlag{ - Name: authApplicationFlagLogoutURI, - Usage: "updated logout uri for the oauth application", + Name: oauthAppFlagLogoutURI, + Usage: "updated logout uri for the OAuth application", Required: false, }, &cli.StringSliceFlag{ - Name: authApplicationFlagEnabledGrants, - Usage: "updated comma separated enabled grants for the auth application. values can be of " + + Name: oauthAppFlagEnabledGrants, + Usage: "updated comma separated enabled grants for the OAuth application. values can be of " + allEnumValues(enabledGrantPrefix, apppb.EnabledGrant_value), Required: false, }, diff --git a/cli/client.go b/cli/client.go index 370f77ee3d7..30e8d5d20c4 100644 --- a/cli/client.go +++ b/cli/client.go @@ -2108,7 +2108,7 @@ func (c *viamClient) updateOAuthAppAction(cCtx *cli.Context, args updateOAuthApp return err } - printf(cCtx.App.Writer, "Successfully updated oauth application %s", args.ClientID) + printf(cCtx.App.Writer, "Successfully updated OAuth app %s", args.ClientID) return nil } @@ -2126,23 +2126,23 @@ func createUpdateOAuthAppRequest(args updateOAuthAppArgs) (*apppb.UpdateOAuthApp clientAuthenticationEnum, ok := apppb.ClientAuthentication_value[clientAuthenticationPrefix+strings.ToUpper(clientAuthentication)] if !ok { - return nil, errors.Errorf("%s must be a valid ClientAuthentication, got %s. "+ + return nil, errors.Errorf("--%s must be a valid ClientAuthentication, got %s. "+ "See `viam organizations auth-service oauth-app update --help` for supported options", - authApplicationFlagClientAuthentication, clientAuthentication) + oauthAppFlagClientAuthentication, clientAuthentication) } pkceEnum, ok := apppb.PKCE_value[pkcePrefix+strings.ToUpper(pkce)] if !ok { - return nil, errors.Errorf("%s must be a valid PKCE, got %s. "+ + return nil, errors.Errorf("--%s must be a valid PKCE, got %s. "+ "See `viam organizations auth-service oauth-app update --help` for supported options", - authApplicationFlagPKCE, pkce) + oauthAppFlagPKCE, pkce) } urlValidationEnum, ok := apppb.URLValidation_value[urlValidationPrefix+strings.ToUpper(urlValidation)] if !ok { - return nil, errors.Errorf("%s must be a valid UrlValidation, got %s. "+ + return nil, errors.Errorf("--%s must be a valid UrlValidation, got %s. "+ "See `viam organizations auth-service oauth-app update --help` for supported options", - authApplicationFlagURLValidation, urlValidation) + oauthAppFlagURLValidation, urlValidation) } egProto, err := enabledGrantsToProto(enabledGrants) @@ -2177,7 +2177,7 @@ func enabledGrantsToProto(enabledGrants []string) ([]apppb.EnabledGrant, error) if !ok { return nil, errors.Errorf("%s must consist of valid EnabledGrants, got %s. "+ "See `viam organizations auth-service oauth-app update --help` for supported options", - authApplicationFlagEnabledGrants, eg) + oauthAppFlagEnabledGrants, eg) } enabledGrantsProto[i] = apppb.EnabledGrant(enum) } diff --git a/cli/client_test.go b/cli/client_test.go index 1eb40a488b9..f215f5b5688 100644 --- a/cli/client_test.go +++ b/cli/client_test.go @@ -1012,20 +1012,54 @@ func TestUpdateOAuthAppAction(t *testing.T) { UpdateOAuthAppFunc: updateOAuthAppFunc, } - flags := make(map[string]any) - flags[generalFlagOrgID] = "org-id" - flags[authApplicationFlagClientID] = "client-id" - flags[authApplicationFlagClientName] = "client-name" - flags[authApplicationFlagClientAuthentication] = "required" - flags[authApplicationFlagURLValidation] = "allow_wildcards" - flags[authApplicationFlagPKCE] = "not_required" - flags[authApplicationFlagOriginURIs] = []string{"https://woof.com/login", "https://arf.com/"} - flags[authApplicationFlagRedirectURIs] = []string{"https://woof.com/home", "https://arf.com/home"} - flags[authApplicationFlagLogoutURI] = "https://woof.com/logout" - flags[authApplicationFlagEnabledGrants] = []string{"implicit", "password"} - cCtx, ac, out, errOut := setup(asc, nil, nil, nil, flags, "token") - - test.That(t, ac.updateOAuthAppAction(cCtx, parseStructFromCtx[updateOAuthAppArgs](cCtx)), test.ShouldBeNil) - test.That(t, len(errOut.messages), test.ShouldEqual, 0) - test.That(t, out.messages[1], test.ShouldContainSubstring, "Successfully updated oauth application") + t.Run("valid inputs", func(t *testing.T) { + flags := make(map[string]any) + flags[generalFlagOrgID] = "org-id" + flags[oauthAppFlagClientID] = "client-id" + flags[oauthAppFlagClientName] = "client-name" + flags[oauthAppFlagClientAuthentication] = "required" + flags[oauthAppFlagURLValidation] = "allow_wildcards" + flags[oauthAppFlagPKCE] = "not_required" + flags[oauthAppFlagOriginURIs] = []string{"https://woof.com/login", "https://arf.com/"} + flags[oauthAppFlagRedirectURIs] = []string{"https://woof.com/home", "https://arf.com/home"} + flags[oauthAppFlagLogoutURI] = "https://woof.com/logout" + flags[oauthAppFlagEnabledGrants] = []string{"implicit", "password"} + cCtx, ac, out, errOut := setup(asc, nil, nil, nil, flags, "token") + test.That(t, ac.updateOAuthAppAction(cCtx, parseStructFromCtx[updateOAuthAppArgs](cCtx)), test.ShouldBeNil) + test.That(t, len(errOut.messages), test.ShouldEqual, 0) + test.That(t, out.messages[0], test.ShouldContainSubstring, "Successfully updated OAuth app") + }) + + t.Run("should error if client-authentication is not a valid enum value", func(t *testing.T) { + flags := make(map[string]any) + flags[generalFlagOrgID] = "org-id" + flags[oauthAppFlagClientID] = "client-id" + flags[oauthAppFlagClientAuthentication] = "not_one_of_the_allowed_values" + cCtx, ac, out, _ := setup(asc, nil, nil, nil, flags, "token") + err := ac.updateOAuthAppAction(cCtx, parseStructFromCtx[updateOAuthAppArgs](cCtx)) + test.That(t, err, test.ShouldNotBeNil) + test.That(t, err.Error(), test.ShouldContainSubstring, "client-authentication must be a valid ClientAuthentication") + test.That(t, len(out.messages), test.ShouldEqual, 0) + }) + + t.Run("should error if url-validation is not a valid enum value", func(t *testing.T) { + flags := map[string]any{oauthAppFlagClientAuthentication: unspecified, oauthAppFlagPKCE: "not_one_of_the_allowed_values"} + cCtx, ac, out, _ := setup(asc, nil, nil, nil, flags, "token") + err := ac.updateOAuthAppAction(cCtx, parseStructFromCtx[updateOAuthAppArgs](cCtx)) + test.That(t, err, test.ShouldNotBeNil) + test.That(t, err.Error(), test.ShouldContainSubstring, "pkce must be a valid PKCE") + test.That(t, len(out.messages), test.ShouldEqual, 0) + }) + + t.Run("should error if url-validation is not a valid enum value", func(t *testing.T) { + flags := map[string]any{ + oauthAppFlagClientAuthentication: unspecified, oauthAppFlagPKCE: unspecified, + oauthAppFlagURLValidation: "not_one_of_the_allowed_values", + } + cCtx, ac, out, _ := setup(asc, nil, nil, nil, flags, "token") + err := ac.updateOAuthAppAction(cCtx, parseStructFromCtx[updateOAuthAppArgs](cCtx)) + test.That(t, err, test.ShouldNotBeNil) + test.That(t, err.Error(), test.ShouldContainSubstring, "url-validation must be a valid UrlValidation") + test.That(t, len(out.messages), test.ShouldEqual, 0) + }) }