Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AUTOMATE-2866] Remove v1 users APIs from gateway #2922

Merged
merged 1 commit into from
Feb 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .bldr.toml
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,6 @@ paths = [
"components/applications-service/*",
"components/authz-service/*",
"components/automate-deployment/*",
"components/automate-gateway/api/auth/*",
"components/automate-gateway/api/authz/*",
"components/automate-gateway/api/compliance/*",
"components/automate-gateway/api/iam/*",
Expand Down
8 changes: 4 additions & 4 deletions components/automate-cli/cmd/chef-automate/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -994,23 +994,23 @@ func runCreateIAMDevUsersCmd(*cobra.Command, []string) error {
return err
}
for username, data := range map[string]struct {
displayName, password, team string
displayName, password, teamID string
}{
"viewer": {"Viewer User", "chefautomate", "viewers"},
"editor": {"Editor User", "chefautomate", "editors"},
} {
userID, _, err := adminmgmt.CreateUserOrUpdatePassword(ctx,
apiClient, username, data.displayName, data.password, false /* dry run */)
apiClient, username, data.displayName, data.password, false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still need the dry run parameter? if we do let's keep the comment so we remember what that bool is for πŸ˜…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree that this comment is useful. The function signature says what the boolean is doing. We don't have similar comments everywhere else we pass a boolean.

if err != nil {
return err
}
// Note: the teams SHOULD exist. But since you never know what happens in a
// long running acceptance env, we'll better ensure them:
teamID, _, err := adminmgmt.EnsureTeam(ctx, data.team, data.team /* description */, apiClient, false /* dry run */)
_, err = adminmgmt.EnsureTeam(ctx, data.teamID, data.teamID, apiClient, false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still need to ensure these teams? How is CreateIAMDevUsers being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least one integration test. I didn't look into it too heavily besides just not breaking it:

|Tylers-MacBook-Pro automate|++> git grep create-iam-dev-users
.studiorc:  chef-automate dev create-iam-dev-users
components/automate-cli/cmd/chef-automate/dev.go:               Use:   "create-iam-dev-users",
integration/tests/iam_v2.sh:    chef-automate dev create-iam-dev-users || return 1
terraform/test-environments/modules/chef_automate_install/templates/install_chef_automate_cli.sh.tpl:chef-automate dev create-iam-dev-users

if err != nil {
return err
}
_, err = adminmgmt.AddUserToTeam(ctx, apiClient, teamID, userID, false /* dry run */)
_, err = adminmgmt.AddUserToTeam(ctx, apiClient, data.teamID, userID, false)
if err != nil {
return err
}
Expand Down
60 changes: 13 additions & 47 deletions components/automate-cli/cmd/chef-automate/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/golang/protobuf/ptypes/wrappers"
"github.com/spf13/cobra"

authz_constants "github.com/chef/automate/components/authz-service/constants"
v2_constants "github.com/chef/automate/components/authz-service/constants/v2"
"github.com/chef/automate/components/automate-cli/pkg/adminmgmt"
"github.com/chef/automate/components/automate-cli/pkg/client/apiclient"
Expand All @@ -18,6 +17,8 @@ import (
iam_req "github.com/chef/automate/components/automate-gateway/api/iam/v2/request"
)

const adminsID = "admins"

var iamCmdFlags = struct {
dryRun bool
adminToken bool
Expand Down Expand Up @@ -157,7 +158,7 @@ func runRestoreDefaultAdminAccessAdminCmd(cmd *cobra.Command, args []string) err
}

// restore admin user and team if needed
userID, adminUserFound, err := adminmgmt.CreateAdminUserOrUpdatePassword(ctx,
membershipID, adminUserFound, err := adminmgmt.CreateAdminUserOrUpdatePassword(ctx,
apiClient, newAdminPassword, iamCmdFlags.dryRun)
if err != nil {
return err
Expand All @@ -169,8 +170,7 @@ func runRestoreDefaultAdminAccessAdminCmd(cmd *cobra.Command, args []string) err
writer.Success("Created new admin user with specified password")
}

adminsTeamID, adminsTeamFound, err := adminmgmt.CreateAdminTeamIfMissing(ctx,
apiClient, iamCmdFlags.dryRun)
adminsTeamFound, err := adminmgmt.CreateAdminTeamIfMissing(ctx, apiClient, iamCmdFlags.dryRun)
if err != nil {
return err
}
Expand All @@ -183,11 +183,11 @@ func runRestoreDefaultAdminAccessAdminCmd(cmd *cobra.Command, args []string) err

// In dry-run mode, we might be missing some IDs that would have been created.
// We'll only hit this condition in dry-run mode.
if iamCmdFlags.dryRun && (userID == "" || adminsTeamID == "") {
if iamCmdFlags.dryRun && (membershipID == "" || !adminsTeamFound) {
writer.Success("Added admin user to admins team")
} else { // non-dry-run mode or dry-run mode where user and team already existed.
userAdded, err := adminmgmt.AddAdminUserToTeam(ctx,
apiClient, adminsTeamID, userID, iamCmdFlags.dryRun)
apiClient, adminsID, membershipID, iamCmdFlags.dryRun)
if err != nil {
return err
}
Expand All @@ -199,52 +199,18 @@ func runRestoreDefaultAdminAccessAdminCmd(cmd *cobra.Command, args []string) err
}
}

// grant access to admins team if needed
resp, err := apiClient.PoliciesClient().GetPolicyVersion(ctx, &iam_req.GetPolicyVersionReq{})
foundAdminsTeaminV2AdminPolicy, err := adminmgmt.UpdateAdminsPolicyIfNeeded(ctx,
apiClient, iamCmdFlags.dryRun)
if err != nil {
return status.Wrap(err, status.APIError, "Failed to verify IAM version")
return err
}

writer.Titlef("Checking IAM %s policies for admin policy with admins team.\n", display(resp.Version))

switch resp.Version.Major {
case iam_common.Version_V1:
foundV1AdminPolicy, createdNewV1Policy, err := adminmgmt.UpdateV1AdminsPolicyIfNeeded(ctx,
apiClient, iamCmdFlags.dryRun)
if err != nil {
return err
}

if foundV1AdminPolicy {
writer.Skipped("Found admin policy that contains the admins team")
} else {
// Note: (tc) This should never happen currently since we currently don't support
// editing policies but adding for future-proofing against the functionality.
// Note: (sr) PurgeSubjectFromPolicies can alter policies -- when a user or a
// team is removed; so, this could be more realistic than we think.
writer.Successf("Found default admins team policy but it did not contain "+
"the admins team subject (%s). Added admins team to default admin policy.",
authz_constants.LocalAdminsTeamSubject)
}
if createdNewV1Policy {
writer.Success("Created new admins policy")
}
case iam_common.Version_V2:
foundAdminsTeaminV2AdminPolicy, err := adminmgmt.UpdateV2AdminsPolicyIfNeeded(ctx,
apiClient, iamCmdFlags.dryRun)
if err != nil {
return err
}

if !foundAdminsTeaminV2AdminPolicy {
writer.Success("Added local team: admins to Chef-managed policy: Admin")
}

writer.Skipped("Found local team: admins in Chef-managed policy: Admin")
default:
// do nothing
if !foundAdminsTeaminV2AdminPolicy {
writer.Success("Added local 'admins' team to Chef-managed 'Administrator' policy")
}

writer.Skipped("Found local 'admins' team in Chef-managed 'Administrator' policy")

if err := apiClient.CloseConnection(); err != nil {
return status.Wrap(err, status.APIUnreachableError, "Failed to close connection to the API")
}
Expand Down
Loading