feat(gcp): enable organization validation #2133
Conversation
Before this commit, users could specify a hardcoded list of project IDs to restrict access to the GCP provisioner. While this works, it can be both toilsome to the team maintaining the Smallstep installation and unintuitive to the internal infrastructure users that may encounter errors as a result of their project not being added. This commit is a rough attempt at adding support for validating that a GCP project belongs to a given GCP organization. It does this by using the `projects.getAncestry` call in the Cloud Resource Manager API. If a token's project claim does not have the given organization ID as its topmost ancestor, the token is rejected. This will require the `resourcemanager.projects.get` IAM permission on the organization. The new `OrganizationID` configuration directive is compatible with the existing `ProjectIDs` configuration. If `ProjectIDs` is non-empty, it will take precedence over the `OrganizationID` and act as it did before, with the minor difference that if `OrganizationID` is also non-empty, the provisioner will check the project's ancestry before rejecting the token. There are a couple outstanding questions and tasks after this commit. I tried to strike the right balance between production-ready and proof-of-concept here, so I'm open to any suggestions. - Is the `authority/provisioner/gcp` package the right place for adding this functionality? Is the new struct the right approach? - We should add tests for validating the organization ID. - How should users configure the authentication for the Cloud Resource Manager client? I expect this would be similar to the Cloud KMS integration. - Does Smallstep Professional run in an environment that will be able to authenticate with Google? We would need to either grant permissions to a Smallstep-owned Google service account if it's run in GCP, or set up something like Google's Workload Identity Federation to handle a K8s, AWS, or Azure deployment.
github-actions
bot
added
the
needs triage
| err := p.ProjectValidator.ValidateProject(ctx, projectID) | ||
|
|
||
| if p.OrganizationID == "" { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Why is the project validation only enforced when the organizationID is not set?
There was a problem hiding this comment.
Good question - because I structured these as complementary, not mutually exclusive. I'm open to changing that, but for now it's implemented as "if you're not in the project list but an organization ID is set and you're in the organization, you're okay". I believe this would allow an incremental adoption for users already using the project ID list feature.
There was a problem hiding this comment.
It makes more sense to return an error even if the organization id is set.
I don't know if they are mutually exclusive, but if the project id is in the token and it doesn't match the ones in my configuration, I would expect an error.
There was a problem hiding this comment.
I can make that change, but let me explain - if you set the projectIDs and organizationID, would you not be confused that it was rejected when a token was in fact inside the organization?
That's what I meant by them being mutually exclusive: if it worked the way you described, you could set either projectIDs or organizationID, but not both. I was thinking about how if someone had set projectIDs today, and wanted to try out the organizationID setting, rather than completely disabling projectIDs and enabling organizationID, they could enable organizationID, try it out with a few projects, and then remove projectIDs.
Or maybe they want to allow their organization and one or a few projects from a separate organization? This would also require them to be complementary.
Again though, this is not something I personally feel strongly about, but my hunch is that this behavior is more user-friendly.
There was a problem hiding this comment.
Hi @ericnorris, I'm not convinced, I still think both must match if they are set. To try the organization setting, you can create a new provisioner without project IDs.
Or maybe they want to allow their organization and one or a few projects from a separate organization? This would also require them to be complementary.
If they want to allow some projects from one organization, but all the ones in another organization. They should create two provisioners, one with the projects with or without organization, and a second one with just the second organization.
If we want to allow two organizations, with your change we will need two provisioners, I'm ok with that. At some point, we can also change the settings to be something like the multiString in
certificates/authority/config/types.go
Line 11 in cf1ef2a
A type that accepts both string and []string. So it can be backward compatible.
There was a problem hiding this comment.
Hi @ericnorris, I'm not convinced, I still think both must match if they are set. To try the organization setting, you can create a new provisioner without project IDs.
Fair, but then should we make this option officially mutually exclusive with projectIDs? Why even set the organization ID if you've set a project ID list? The former is what actually matters, and allowing organizationID to be set at the same time is misleading since it's redundant.
There was a problem hiding this comment.
As the project ID is globally unique, yes it makes sense to not use both at the same time. But I don't mind to check both. I leave that decission to you.
Hey all, I'm submitting a PR to enable validating that a project is a part of a GCP organization, rather than a static list of project IDs. As I mention in the commit message, I tried to strike the right balance between production-ready and proof-of-concept, so feel free to leave as much feedback as possible since I'm open to changing anything.
I'm going to share the first commit message below:
Before this commit, users could specify a hardcoded list of project IDs to restrict access to the GCP provisioner. While this works, it can be both toilsome to the team maintaining the Smallstep installation and unintuitive to the internal infrastructure users that may encounter errors as a result of their project not being added.
This commit is a rough attempt at adding support for validating that a GCP project belongs to a given GCP organization. It does this by using the
projects.getAncestrycall in the Cloud Resource Manager API. If a token's project claim does not have the given organization ID as its topmost ancestor, the token is rejected. This will require theresourcemanager.projects.getIAM permission on the organization.The new
OrganizationIDconfiguration directive is compatible with the existingProjectIDsconfiguration. IfProjectIDsis non-empty, it will take precedence over theOrganizationIDand act as it did before, with the minor difference that ifOrganizationIDis also non-empty, the provisioner will check the project's ancestry before rejecting the token.There are a couple outstanding questions and tasks after this commit. I tried to strike the right balance between production-ready and proof-of-concept here, so I'm open to any suggestions.
authority/provisioner/gcppackage the right place for adding this functionality? Is the new struct the right approach?