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

feat: infra view role #1507

Merged
merged 1 commit into from
Apr 7, 2022
Merged

feat: infra view role #1507

merged 1 commit into from
Apr 7, 2022

Conversation

BruceMacD
Copy link
Collaborator

  • allow identities with the view role read-only access to server state

Summary

There should be a way to audit the state of infra. Add a "view" role to enable this.

Also added some specific role checking tests in passing.

Checklist

  • Wrote appropriate unit tests
  • Considered security implications of the change
  • Updated associated docs where necessary
  • Updated associated configuration where necessary
  • Change is backwards compatible if it needs to be (user can upgrade without manual steps?)
  • Nothing sensitive logged
  • Commit message conforms to Conventional Commit
  • GitHub Actions are passing
  • Considered data migrations for smooth upgrades

Related Issues

Resolves #1505

- allow identities with the view role read-only access to server state
@@ -22,7 +22,7 @@ func currentAccessKey(c *gin.Context) *models.AccessKey {
}

func ListAccessKeys(c *gin.Context, identityID uid.ID, name string) ([]models.AccessKey, error) {
db, err := RequireInfraRole(c, models.InfraAdminRole)
db, err := RequireInfraRole(c, models.InfraAdminRole, models.InfraViewRole)
Copy link
Collaborator Author

@BruceMacD BruceMacD Apr 7, 2022

Choose a reason for hiding this comment

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

This doesn't output the key secrets. Only key names, expires and who they were issued for.

@mxyng
Copy link
Collaborator

mxyng commented Apr 7, 2022

I feel like with this role, we don't need the infra user role. The default could be either no role for new user where they can only access they resources they own, the view role where they can view but not edit all resources, and admin where they have full access to all resource. Thoughts?

@BruceMacD
Copy link
Collaborator Author

BruceMacD commented Apr 7, 2022

I feel like with this role, we don't need the infra user role. The default could be either no role for new user where they can only access they resources they own, the view role where they can view but not edit all resources, and admin where they have full access to all resource. Thoughts?

Yeah, getting rid of the user role is definitely ideal. I think we should do with something like #1084, the destinations grant checks will be needed.

@BruceMacD BruceMacD merged commit 00a47e1 into main Apr 7, 2022
@BruceMacD BruceMacD deleted the view-role branch April 7, 2022 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an infra view role
2 participants