From e21e462f000a957e012d4506e6e96fafa91fd96d Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Sat, 13 Jan 2024 16:27:05 +0100 Subject: [PATCH] Auth: Improve "auth add" and "client add" CLI commands #808 #3943 Signed-off-by: Michael Mayer --- internal/commands/auth_add.go | 52 ++++++++++++++++--------- internal/commands/clients.go | 29 +++++++------- internal/commands/clients_add.go | 4 +- internal/entity/auth_client_fixtures.go | 6 +-- internal/entity/auth_client_test.go | 4 +- internal/form/client.go | 7 +--- pkg/authn/client.go | 2 +- pkg/authn/methods.go | 14 +++++-- pkg/authn/methods_test.go | 8 ++-- pkg/rnd/name.go | 5 ++- pkg/rnd/name_test.go | 15 +++++-- 11 files changed, 90 insertions(+), 56 deletions(-) diff --git a/internal/commands/auth_add.go b/internal/commands/auth_add.go index 5be14b191..4218bfd24 100644 --- a/internal/commands/auth_add.go +++ b/internal/commands/auth_add.go @@ -17,16 +17,15 @@ import ( var AuthAddFlags = []cli.Flag{ cli.StringFlag{ Name: "name, n", - Usage: "arbitrary `IDENTIFIER` for the new access token", + Usage: "arbitrary name to help identify the access `TOKEN`", }, cli.StringFlag{ Name: "user, u", - Usage: "provide a `USERNAME` if a personal access token for a specific user account should be created", + Usage: "`USERNAME` of the account the access token belongs to (leave empty for none)", }, cli.StringFlag{ Name: "scope, s", Usage: "authorization `SCOPE` for the access token e.g. \"metrics\" or \"photos albums\" (\"*\" to allow all scopes)", - Value: "*", }, cli.Int64Flag{ Name: "expires, e", @@ -38,7 +37,7 @@ var AuthAddFlags = []cli.Flag{ // AuthAddCommand configures the command name, flags, and action. var AuthAddCommand = cli.Command{ Name: "add", - Usage: "Creates a new client access token and shows it", + Usage: "Creates a new client access token", Flags: AuthAddFlags, Action: authAddAction, } @@ -46,11 +45,23 @@ var AuthAddCommand = cli.Command{ // authAddAction shows detailed session information. func authAddAction(ctx *cli.Context) error { return CallWithDependencies(ctx, func(conf *config.Config) error { - name := ctx.String("name") + // Get username from command flag. + userName := ctx.String("user") - if name == "" { + // Find user account. + user := entity.FindUserByName(userName) + + if user == nil && userName != "" { + return fmt.Errorf("user %s not found", clean.LogQuote(userName)) + } + + // Get token name from command flag or ask for it. + tokenName := ctx.String("name") + + if tokenName == "" { prompt := promptui.Prompt{ - Label: "Token Name", + Label: "Token Name", + Default: rnd.Name(), } res, err := prompt.Run() @@ -59,24 +70,29 @@ func authAddAction(ctx *cli.Context) error { return err } - name = clean.Name(res) + tokenName = clean.Name(res) } - // Set a default token name if no specific name has been provided. - if name == "" { - name = rnd.Name() - } + // Get auth scope from command flag or ask for it. + authScope := ctx.String("scope") - // Username provided? - userName := ctx.String("user") - user := entity.FindUserByName(userName) + if authScope == "" { + prompt := promptui.Prompt{ + Label: "Authorization Scope", + Default: "*", + } - if user == nil && userName != "" { - return fmt.Errorf("user %s not found", clean.LogQuote(userName)) + res, err := prompt.Run() + + if err != nil { + return err + } + + authScope = clean.Scope(res) } // Create client session. - sess, err := entity.CreateClientAccessToken(name, ctx.Int64("expires"), ctx.String("scope"), user) + sess, err := entity.CreateClientAccessToken(tokenName, ctx.Int64("expires"), authScope, user) if err != nil { return fmt.Errorf("failed to create access token: %s", err) diff --git a/internal/commands/clients.go b/internal/commands/clients.go index e859312e7..4ac582c25 100644 --- a/internal/commands/clients.go +++ b/internal/commands/clients.go @@ -1,16 +1,17 @@ package commands import ( - "github.com/photoprism/photoprism/pkg/authn" "github.com/urfave/cli" + + "github.com/photoprism/photoprism/pkg/authn" ) // Usage hints for the client management subcommands. const ( ClientNameUsage = "arbitrary name to help identify the `CLIENT` application" - ClientUserName = "provide a `USERNAME` if the client belongs to a specific user account" - ClientAuthMethod = "supported authentication `METHOD` for the client application" + ClientUserName = "`USERNAME` of the account the client application belongs to (leave empty for none)" ClientAuthScope = "authorization `SCOPE` of the client e.g. \"metrics\" or \"photos albums\" (\"*\" to allow all scopes)" + ClientAuthMethod = "supported authentication `METHOD` for the client application" ClientAuthExpires = "access token lifetime in `SECONDS`, after which a new token must be created by the client (-1 to disable)" ClientAuthTokens = "maximum `NUMBER` of access tokens the client can create (-1 to disable)" ClientRegenerateSecret = "generate a new client secret and display it" @@ -43,15 +44,16 @@ var ClientAddFlags = []cli.Flag{ Name: "user, u", Usage: ClientUserName, }, - cli.StringFlag{ - Name: "method, m", - Usage: ClientAuthMethod, - Value: authn.MethodOAuth2.String(), - }, cli.StringFlag{ Name: "scope, s", Usage: ClientAuthScope, }, + cli.StringFlag{ + Name: "method, m", + Usage: ClientAuthMethod, + Value: authn.MethodOAuth2.String(), + Hidden: true, + }, cli.Int64Flag{ Name: "expires, e", Usage: ClientAuthExpires, @@ -68,15 +70,16 @@ var ClientModFlags = []cli.Flag{ Name: "name, n", Usage: ClientNameUsage, }, - cli.StringFlag{ - Name: "method, m", - Usage: ClientAuthMethod, - Value: authn.MethodOAuth2.String(), - }, cli.StringFlag{ Name: "scope, s", Usage: ClientAuthScope, }, + cli.StringFlag{ + Name: "method, m", + Usage: ClientAuthMethod, + Value: authn.MethodOAuth2.String(), + Hidden: true, + }, cli.Int64Flag{ Name: "expires, e", Usage: ClientAuthExpires, diff --git a/internal/commands/clients_add.go b/internal/commands/clients_add.go index 11e449826..94fa5a9a1 100644 --- a/internal/commands/clients_add.go +++ b/internal/commands/clients_add.go @@ -13,6 +13,7 @@ import ( "github.com/photoprism/photoprism/pkg/clean" "github.com/photoprism/photoprism/pkg/list" "github.com/photoprism/photoprism/pkg/report" + "github.com/photoprism/photoprism/pkg/rnd" ) // ClientsAddCommand configures the command name, flags, and action. @@ -39,7 +40,8 @@ func clientsAddAction(ctx *cli.Context) error { if interactive && frm.ClientName == "" { prompt := promptui.Prompt{ - Label: "Client Name", + Label: "Client Name", + Default: rnd.Name(), } res, err := prompt.Run() diff --git a/internal/entity/auth_client_fixtures.go b/internal/entity/auth_client_fixtures.go index 8be20d82a..a29ee33e0 100644 --- a/internal/entity/auth_client_fixtures.go +++ b/internal/entity/auth_client_fixtures.go @@ -43,11 +43,11 @@ var ClientFixtures = ClientMap{ UserName: UserFixtures.Pointer("bob").UserName, user: UserFixtures.Pointer("bob"), ClientName: "Bob", - ClientType: authn.ClientWebDAV, + ClientType: authn.ClientPublic, ClientURL: "", CallbackURL: "", - AuthMethod: authn.MethodBasic.String(), - AuthScope: "webdav files photos", + AuthMethod: authn.MethodOAuth2.String(), + AuthScope: "*", AuthExpires: 0, AuthTokens: -1, AuthEnabled: false, diff --git a/internal/entity/auth_client_test.go b/internal/entity/auth_client_test.go index ac8656777..3b4e9fb25 100644 --- a/internal/entity/auth_client_test.go +++ b/internal/entity/auth_client_test.go @@ -368,7 +368,7 @@ func TestClient_SetFormValues(t *testing.T) { t.Fatal(err) } - var values = form.Client{ClientName: "Annika", AuthMethod: authn.MethodBasic.String(), + var values = form.Client{ClientName: "Annika", AuthMethod: authn.MethodOAuth2.String(), AuthScope: "metrics", AuthExpires: -4000, AuthTokens: -5, @@ -389,7 +389,7 @@ func TestClient_SetFormValues(t *testing.T) { } var values = form.Client{ClientName: "Friend", - AuthMethod: authn.MethodBasic.String(), + AuthMethod: authn.MethodOAuth2.String(), AuthScope: "test", AuthExpires: 4000000, AuthTokens: 3000000000, diff --git a/internal/form/client.go b/internal/form/client.go index efb76d44f..c4f945505 100644 --- a/internal/form/client.go +++ b/internal/form/client.go @@ -40,12 +40,7 @@ func NewClientFromCli(ctx *cli.Context) Client { f.ClientName = clean.Name(ctx.String("name")) f.AuthScope = clean.Scope(ctx.String("scope")) - - if method := clean.Scope(ctx.String("method")); authn.MethodOAuth2.Equal(method) { - f.AuthMethod = authn.MethodOAuth2.String() - } else if authn.MethodBasic.Equal(method) { - f.AuthMethod = authn.MethodBasic.String() - } + f.AuthMethod = authn.Method(ctx.String("method")).String() if authn.MethodOAuth2.NotEqual(f.AuthMethod) { f.AuthScope = "webdav" diff --git a/pkg/authn/client.go b/pkg/authn/client.go index f54a468a8..93cf97881 100644 --- a/pkg/authn/client.go +++ b/pkg/authn/client.go @@ -3,6 +3,6 @@ package authn // API client types. const ( ClientConfidential = "confidential" - ClientWebDAV = "webdav" + ClientPublic = "public" ClientUnknown = "" ) diff --git a/pkg/authn/methods.go b/pkg/authn/methods.go index 07fb7c4b5..95ed0dbcb 100644 --- a/pkg/authn/methods.go +++ b/pkg/authn/methods.go @@ -13,10 +13,10 @@ type MethodType string // Authentication methods. const ( MethodDefault MethodType = "default" - MethodBasic MethodType = "basic" MethodAccessToken MethodType = "access_token" MethodOAuth2 MethodType = "oauth2" MethodOIDC MethodType = "oidc" + Method2FA MethodType = "2fa" MethodUnknown MethodType = "" ) @@ -30,10 +30,12 @@ func (t MethodType) String() string { switch t { case "": return string(MethodDefault) - case "openid": - return string(MethodOIDC) case "oauth": return string(MethodOAuth2) + case "openid": + return string(MethodOIDC) + case "totp": + return string(Method2FA) default: return string(t) } @@ -58,6 +60,8 @@ func (t MethodType) Pretty() string { return "OAuth2" case MethodOIDC: return "OIDC" + case Method2FA: + return "2FA" default: return txt.UpperFirst(t.String()) } @@ -70,6 +74,10 @@ func Method(s string) MethodType { return MethodDefault case "oauth2", "oauth": return MethodOAuth2 + case "sso": + return MethodOIDC + case "two-factor", "totp": + return Method2FA default: return MethodType(clean.TypeLower(s)) } diff --git a/pkg/authn/methods_test.go b/pkg/authn/methods_test.go index 789d095ff..5837abb6c 100644 --- a/pkg/authn/methods_test.go +++ b/pkg/authn/methods_test.go @@ -8,35 +8,35 @@ import ( func TestMethodType_String(t *testing.T) { assert.Equal(t, "default", MethodDefault.String()) - assert.Equal(t, "basic", MethodBasic.String()) assert.Equal(t, "access_token", MethodAccessToken.String()) assert.Equal(t, "oauth2", MethodOAuth2.String()) assert.Equal(t, "oidc", MethodOIDC.String()) + assert.Equal(t, "2fa", Method2FA.String()) assert.Equal(t, "default", MethodUnknown.String()) } func TestMethodType_IsDefault(t *testing.T) { assert.Equal(t, true, MethodDefault.IsDefault()) - assert.Equal(t, false, MethodBasic.IsDefault()) assert.Equal(t, false, MethodAccessToken.IsDefault()) assert.Equal(t, false, MethodOAuth2.IsDefault()) assert.Equal(t, false, MethodOIDC.IsDefault()) + assert.Equal(t, false, Method2FA.IsDefault()) assert.Equal(t, true, MethodUnknown.IsDefault()) } func TestMethodType_Pretty(t *testing.T) { assert.Equal(t, "Default", MethodDefault.Pretty()) - assert.Equal(t, "Basic", MethodBasic.Pretty()) assert.Equal(t, "Access Token", MethodAccessToken.Pretty()) assert.Equal(t, "OAuth2", MethodOAuth2.Pretty()) assert.Equal(t, "OIDC", MethodOIDC.Pretty()) + assert.Equal(t, "2FA", Method2FA.Pretty()) assert.Equal(t, "Default", MethodUnknown.Pretty()) } func TestMethod(t *testing.T) { assert.Equal(t, MethodDefault, Method("default")) - assert.Equal(t, MethodBasic, Method("basic")) assert.Equal(t, MethodAccessToken, Method("access_token")) assert.Equal(t, MethodOAuth2, Method("oauth2")) assert.Equal(t, MethodOIDC, Method("oidc")) + assert.Equal(t, Method2FA, Method("2fa")) } diff --git a/pkg/rnd/name.go b/pkg/rnd/name.go index f04dc6505..c45fcbf17 100644 --- a/pkg/rnd/name.go +++ b/pkg/rnd/name.go @@ -1,6 +1,9 @@ package rnd import ( + "golang.org/x/text/cases" + "golang.org/x/text/language" + petname "github.com/dustinkirkland/golang-petname" ) @@ -11,5 +14,5 @@ func Name() string { // NameN returns a pronounceable name consisting of a random combination of adverbs, an adjective, and a pet name. func NameN(n int) string { - return petname.Generate(n, "-") + return cases.Title(language.English, cases.Compact).String(petname.Generate(n, " ")) } diff --git a/pkg/rnd/name_test.go b/pkg/rnd/name_test.go index 3d7a10135..ba739dceb 100644 --- a/pkg/rnd/name_test.go +++ b/pkg/rnd/name_test.go @@ -1,18 +1,22 @@ package rnd import ( + "strings" "testing" "github.com/stretchr/testify/assert" ) func TestName(t *testing.T) { - assert.NotEmpty(t, Name()) + name := Name() + assert.NotEmpty(t, name) + assert.Equal(t, 1, strings.Count(name, " ")) for n := 0; n < 10; n++ { s := Name() t.Logf("Name %d: %s", n, s) - assert.NotEmpty(t, Name()) + assert.NotEmpty(t, s) + assert.Equal(t, 1, strings.Count(s, " ")) } } @@ -23,11 +27,14 @@ func BenchmarkName(b *testing.B) { } func TestNameN(t *testing.T) { - assert.NotEmpty(t, NameN(2)) + name := NameN(2) + assert.NotEmpty(t, name) + assert.Equal(t, 1, strings.Count(name, " ")) for n := 0; n < 10; n++ { s := NameN(n + 1) t.Logf("NameN %d: %s", n, s) - assert.NotEmpty(t, Name()) + assert.NotEmpty(t, s) + assert.Equal(t, n, strings.Count(s, " ")) } }