Skip to content

Commit

Permalink
Allow more profile commands to be run while unauthenticated
Browse files Browse the repository at this point in the history
Some of the profile/profiles commands required the CLI to be
authenticated unnecessarily. This fixes that and makes authentication
optional for `profile set` when validating the org/project_id.
  • Loading branch information
dadgar committed Mar 26, 2024
1 parent 7f4f73b commit 19c3e90
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 19 deletions.
3 changes: 3 additions & 0 deletions .changelog/47.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
profile: Some profile and profiles commands required authentication unnecessarily.
```
1 change: 1 addition & 0 deletions internal/commands/profile/profiles/activate.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func NewCmdActivate(ctx *cmd.Context) *cmd.Command {
},
},
},
NoAuthRequired: true,
RunF: func(c *cmd.Command, args []string) error {
opts.Name = args[0]
l, err := profile.NewLoader()
Expand Down
1 change: 1 addition & 0 deletions internal/commands/profile/profiles/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func NewCmdCreate(ctx *cmd.Context) *cmd.Command {
},
},
},
NoAuthRequired: true,
RunF: func(c *cmd.Command, args []string) error {
opts.Name = args[0]
l, err := profile.NewLoader()
Expand Down
1 change: 1 addition & 0 deletions internal/commands/profile/profiles/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func NewCmdDelete(ctx *cmd.Context) *cmd.Command {
`),
},
},
NoAuthRequired: true,
Args: cmd.PositionalArguments{
Autocomplete: predictProfiles(true, false),
Args: []cmd.PositionalArgument{
Expand Down
1 change: 1 addition & 0 deletions internal/commands/profile/profiles/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func NewCmdList(ctx *cmd.Context) *cmd.Command {
Command: "$ hcp profile profiles list",
},
},
NoAuthRequired: true,
RunF: func(c *cmd.Command, args []string) error {
l, err := profile.NewLoader()
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion internal/commands/profile/profiles/rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func NewCmdRename(ctx *cmd.Context) *cmd.Command {
Examples: []cmd.Example{
{
Preamble: heredoc.New(ctx.IO).Must(`
To rename profile {{ template "mdCodeOrBold" "my-profile" }} to
To rename profile {{ template "mdCodeOrBold" "my-profile" }} to
{{ template "mdCodeOrBold" "new-profile" }}, run:
`),
Command: "$ hcp profile profiles rename my-profile --new-name=new-profile",
Expand All @@ -51,6 +51,7 @@ func NewCmdRename(ctx *cmd.Context) *cmd.Command {
},
},
},
NoAuthRequired: true,
RunF: func(c *cmd.Command, args []string) error {
opts.ExistingName = args[0]
l, err := profile.NewLoader()
Expand Down
25 changes: 22 additions & 3 deletions internal/commands/profile/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/hashicorp/hcp-sdk-go/clients/cloud-resource-manager/stable/2019-12-10/client/organization_service"
"github.com/hashicorp/hcp-sdk-go/clients/cloud-resource-manager/stable/2019-12-10/client/project_service"
"github.com/hashicorp/hcp/internal/pkg/auth"
"github.com/hashicorp/hcp/internal/pkg/cmd"
"github.com/hashicorp/hcp/internal/pkg/heredoc"
"github.com/hashicorp/hcp/internal/pkg/iostreams"
Expand All @@ -24,6 +25,7 @@ func NewCmdSet(ctx *cmd.Context) *cmd.Command {
Profile: ctx.Profile,
ProjectService: project_service.New(ctx.HCP, nil),
OrganizationService: organization_service.New(ctx.HCP, nil),
isAuthed: auth.IsAuthenticated,
}

cmd := &cmd.Command{
Expand Down Expand Up @@ -69,6 +71,7 @@ func NewCmdSet(ctx *cmd.Context) *cmd.Command {
AdditionalDocs: []cmd.DocSection{
availablePropertiesDoc(ctx.IO),
},
NoAuthRequired: true,
RunF: func(c *cmd.Command, args []string) error {
opts.Property = args[0]
opts.Value = args[1]
Expand All @@ -79,11 +82,15 @@ func NewCmdSet(ctx *cmd.Context) *cmd.Command {
return cmd
}

type isAuthedFn func() (bool, error)

type SetOpts struct {
Ctx context.Context
IO iostreams.IOStreams
Profile *profile.Profile

isAuthed isAuthedFn

// Resource Manager client
ProjectService project_service.ClientService
OrganizationService organization_service.ClientService
Expand Down Expand Up @@ -168,7 +175,13 @@ func setRun(opts *SetOpts) error {
}

func (o *SetOpts) validateProject() (bool, error) {
if o.Property != "project_id" || !o.IO.CanPrompt() {
// If the CLI is not authenticated, skip validation.
isAuthed, err := o.isAuthed()
if err != nil {
return false, err
}

if o.Property != "project_id" || !o.IO.CanPrompt() || !isAuthed {
return true, nil
}

Expand All @@ -178,7 +191,7 @@ func (o *SetOpts) validateProject() (bool, error) {
ID: o.Value,
Context: o.Ctx,
}
_, err := o.ProjectService.ProjectServiceGet(params, nil)
_, err = o.ProjectService.ProjectServiceGet(params, nil)
if err != nil {
var listErr *project_service.ProjectServiceGetDefault
if errors.As(err, &listErr) {
Expand Down Expand Up @@ -211,7 +224,13 @@ func (o *SetOpts) validateProject() (bool, error) {
}

func (o *SetOpts) validateOrg() (bool, error) {
if o.Property != "organization_id" || !o.IO.CanPrompt() {
// If the CLI is not authenticated, skip validation.
isAuthed, err := o.isAuthed()
if err != nil {
return false, err
}

if o.Property != "organization_id" || !o.IO.CanPrompt() || !isAuthed {
return true, nil
}

Expand Down
48 changes: 33 additions & 15 deletions internal/commands/profile/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func TestSet(t *testing.T) {
ProjectService: nil,
Property: c.Property,
Value: c.Value,
isAuthed: func() (bool, error) { return true, nil },
}

err := setRun(o)
Expand Down Expand Up @@ -113,14 +114,15 @@ func TestSet_Project(t *testing.T) {
Property: "project_id",
}

setup := func(quiet, tty bool, projectID string) {
setup := func(quiet, tty, authed bool, projectID string) {
o.Value = projectID
io.SetQuiet(quiet)
io.InputTTY = tty
io.ErrorTTY = tty
io.Input.Reset()
io.Error.Reset()
io.Output.Reset()
o.isAuthed = func() (bool, error) { return authed, nil }
}

checkProject := func(expected string) {
Expand All @@ -129,9 +131,9 @@ func TestSet_Project(t *testing.T) {
r.Equal(expected, loadedProfile.ProjectID)
}

// Run with quiet off, TTY's, and return that the user has access to the project
// Run with quiet off, TTY's, authenticated, and return that the user has access to the project
{
setup(false, true, "123")
setup(false, true, true, "123")
psvc.EXPECT().ProjectServiceGet(mock.MatchedBy(func(getReq *project_service.ProjectServiceGetParams) bool {
return getReq != nil && getReq.ID == o.Value
}), mock.Anything).Once().Return(nil, nil)
Expand All @@ -141,7 +143,7 @@ func TestSet_Project(t *testing.T) {

// Call again but return permission denied and accept the prompt
{
setup(false, true, "accept")
setup(false, true, true, "accept")
psvc.EXPECT().ProjectServiceGet(mock.MatchedBy(func(getReq *project_service.ProjectServiceGetParams) bool {
return getReq != nil && getReq.ID == o.Value
}), mock.Anything).Once().Return(nil, project_service.NewProjectServiceGetDefault(http.StatusForbidden))
Expand All @@ -158,7 +160,7 @@ func TestSet_Project(t *testing.T) {

// Call again but return permission denied and do not accept the prompt
{
setup(false, true, "no-accept")
setup(false, true, true, "no-accept")
psvc.EXPECT().ProjectServiceGet(mock.MatchedBy(func(getReq *project_service.ProjectServiceGetParams) bool {
return getReq != nil && getReq.ID == o.Value
}), mock.Anything).Once().Return(nil, project_service.NewProjectServiceGetDefault(http.StatusForbidden))
Expand All @@ -175,19 +177,27 @@ func TestSet_Project(t *testing.T) {

// Run again but with quiet
{
setup(true, true, "789")
setup(true, true, true, "789")
r.NoError(setRun(o))
r.NotContains(io.Error.String(), "Are you sure you wish to set the")
checkProject("789")
}

// Run again but with no quiet but no tty
{
setup(false, false, "012")
setup(false, false, true, "012")
r.NoError(setRun(o))
r.NotContains(io.Error.String(), "Are you sure you wish to set the")
checkProject("012")
}

// Run again but unauthenticated
{
setup(true, true, false, "789")
r.NoError(setRun(o))
r.NotContains(io.Error.String(), "Are you sure you wish to set the")
checkProject("789")
}
}

func TestSet_Organization(t *testing.T) {
Expand All @@ -205,14 +215,15 @@ func TestSet_Organization(t *testing.T) {
Property: "organization_id",
}

setup := func(quiet, tty bool, orgID string) {
setup := func(quiet, tty, authed bool, orgID string) {
o.Value = orgID
io.SetQuiet(quiet)
io.InputTTY = tty
io.ErrorTTY = tty
io.Input.Reset()
io.Error.Reset()
io.Output.Reset()
o.isAuthed = func() (bool, error) { return authed, nil }
}

orgResp := func(ids ...string) *organization_service.OrganizationServiceListOK {
Expand All @@ -236,17 +247,17 @@ func TestSet_Organization(t *testing.T) {
r.Equal(expected, loadedProfile.OrganizationID)
}

// Run with quiet off, TTY's, and return the user is a member of the org
// Run with quiet off, TTY's, authenticated, and return the user is a member of the org
{
setup(false, true, "member")
setup(false, true, true, "member")
osvc.EXPECT().OrganizationServiceList(mock.Anything, mock.Anything).Once().Return(orgResp("1", "2", "member"), nil)
r.NoError(setRun(o))
checkOrg("member")
}

// Not be a member, expect prompting and respond no
{
setup(false, true, "not-a-member")
setup(false, true, true, "not-a-member")
osvc.EXPECT().OrganizationServiceList(mock.Anything, mock.Anything).Once().Return(orgResp("1", "2", "123"), nil)

// Answer yes to prompt
Expand All @@ -261,7 +272,7 @@ func TestSet_Organization(t *testing.T) {

// Not be a member, expect prompting and respond yes
{
setup(false, true, "not-a-member")
setup(false, true, true, "not-a-member")
osvc.EXPECT().OrganizationServiceList(mock.Anything, mock.Anything).Once().Return(orgResp("1", "2", "123"), nil)

// Answer yes to prompt
Expand All @@ -276,21 +287,28 @@ func TestSet_Organization(t *testing.T) {

// Do not be a member; but quiet
{
setup(true, true, "not-a-member-quiet")
setup(true, true, true, "not-a-member-quiet")
r.NoError(setRun(o))
checkOrg("not-a-member-quiet")
}

// Do not be a member; but no tty
{
setup(false, false, "not-a-member-no-tty")
setup(false, false, true, "not-a-member-no-tty")
r.NoError(setRun(o))
checkOrg("not-a-member-no-tty")
}

// Do not be a member; but unauthenticated
{
setup(false, false, false, "not-a-member-no-tty")
r.NoError(setRun(o))
checkOrg("not-a-member-no-tty")
}

// Return error
{
setup(false, true, "error")
setup(false, true, true, "error")
osvc.EXPECT().OrganizationServiceList(mock.Anything, mock.Anything).Once().
Return(nil, organization_service.NewOrganizationServiceListDefault(http.StatusInternalServerError))

Expand Down
18 changes: 18 additions & 0 deletions internal/pkg/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"
"path/filepath"
"time"

hcpconf "github.com/hashicorp/hcp-sdk-go/config"
"github.com/mitchellh/go-homedir"
Expand Down Expand Up @@ -65,3 +66,20 @@ func GetHCPCredFilePath(credFileDir string) (string, error) {
credFilePath := filepath.Join(dir, CredFileName)
return credFilePath, nil
}

// IsAuthenticated returns if there is a valid token available.
func IsAuthenticated() (bool, error) {
// Create the HCP Config
hcpCfg, err := GetHCPConfig(hcpconf.WithoutBrowserLogin())
if err != nil {
return false, fmt.Errorf("failed to instantiate HCP config to check authentication status: %w", err)
}

if tkn, err := hcpCfg.Token(); err != nil {
return false, nil
} else if !tkn.Expiry.After(time.Now()) {
return false, nil
}

return true, nil
}

0 comments on commit 19c3e90

Please sign in to comment.