-
Notifications
You must be signed in to change notification settings - Fork 3
feat: dedicated renku git user #21
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
base: main
Are you sure you want to change the base?
Conversation
| ### UI | ||
|
|
||
| There are some changes needed in the UI for this. Namely: | ||
| - To get a list of all projects for a user the UI would have to call Gitlab on behalf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think this might work without any changes as long as we get the user OAuth access token passed to our User's Projects API as we do now. The reason is that we find the list of user's non-active projects by calling the GL User Projects API with both the userId and OAuth token we get from the UI in the request (these two are injected by the Gateway). I mean, at least now this API doesn't use the RenkuBot user (or Project Access Token) and probably it would be hard to achieve the same functionality if it would have to.
| KG integration is not active. It could figure this out for public Gitlab projects | ||
| but it would be impossible to find private projects that are not in the KG. | ||
| - Creating a project on Renku will have to include adding this Renkubot user to the project | ||
| with `Reporter` permissions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might also be that the Renkubot user would be added to the project during the activation call on the KG side. Obviously, it's not a must, but I can see that building a coherent picture and keeping the responsibilities in the proper places. I might be wrong but for me, it's a part of the activation process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the same - that it would be part of the KG activation flow.
| - Creating a project on Renku will have to include adding this Renkubot user to the project | ||
| with `Reporter` permissions. | ||
| - KG integration for a specific project can still be verified with the KG. In some cases | ||
| the KG will simply not know anything at all for a project. In this case KG integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that I see matching nicely to the check we do on GET /api/projects/24541/graph/status. At the moment it returns info about activation status and processing progress.
| - Enabling KG integration on projects that do not have it will have to be modified to also | ||
| add the user to the project similarly to creating a project. | ||
| - Disabling / removing KG integration results in removal of the project from the KG and removing | ||
| the Renkubot user from the project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might be done when we remove a project from KG. At the moment we try to remove the webhook but we could easily try to remove the Renkubot user, too. The only question is whether we can do that using the Renkubot token? It's also worth mentioning, that currently, there's no route of disabling/removing a project from KG.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine the renkubot user could remove itself from a project... ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| ### KG | ||
|
|
||
| Depending on when the Gateway refactoring is completed, the KG may not have to deal with the Renkubot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's not a big difference for the KG which option we'd choose. At least from the implementation side, I think it might be even easier if we'd get the credentials injected so we can simply exchange them for an OAuth token in the token repository.
|
|
||
| ## Drawbacks | ||
|
|
||
| 1. The Renkubot user will have read access to many repos. If the credentials are stolen or leaked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I could see this as a serious concern for some of our users or admins.
The other thing is that we'd need to ensure our Renkubot user has special capabilities (or rather does not have certain capabilities). I'm thinking here about something similar to the Project Access Token bot user which for instance cannot be used to log in. I'm guessing we'd need to create it as this special internal GL user if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the major reason not to implement this RFC imho. The bot user will only have read access to the repos, but that might already be too big of a potential risk. AFAICT, it's not possible to create "internal" users like the project bots that gitlab creates automatically when a project access token is created (ref).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, only recently we were talking about that with Eike again and we ended up with the same conclusion. We have to say we don't feel comfortable adding a single user to all the projects. Imagine the credentials get compromised. I cannot even think of an emergency plan we should follow in such a scenario (I guess we'd need a very carefully prepared plan for the incident). The thing is that if the credentials leak the person would be able to do whatever: log in, use the API and access the repos, remove the original members and so on. The list is almost indefinite. The Project Access Token solution with all the known issues is in my eyes far less problematic. The access token is known only to token-repository and cannot be even viewed by the owner of the project. You cannot use it to log in, either.
Obviously, renkulab.io it's maybe a bit less of an issue as we don't guarantee anything in a written form (as far as I know). But on envs like renku health or BIT? Would it be even easy to communicate that to the responsible people?
Also, what's maybe worth mentioning is the procedure of creation of the super renku user. It might be that we'll need to cede that to the admins with some very explicit message about the importance and power of this user. I wouldn't be surprised if there will be questions about why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jachro I'm not sure I follow. The renku user wouldn't be the admin or maintainer of those projects. It would have the lowest possible read-only permissions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I forgot we don't need this user to have any write access to the repos. Sure, that makes it definitely better but is that good enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would like to visit this topic. Personally I think the risk of credentials leaking is the same for a single user at it is for 20k users.
| then they could be used to read the contents of private repos. | ||
|
|
||
| 2. When this is rolled out we would automatically add the Renkubot user to all projects. This may | ||
| be confusing to users. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, all the repos already have a renkubot user :) However, each repo has a different one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All project maintainers will probably be notified that the bot user has been added. We will need to communicate this clearly to the users so that no one goes and removes it in response.
| 2. Do we implement simultaneously with the gateway refactoring or do we have an initial | ||
| implementation where the KG handles the Renkubot credentials. | ||
|
|
||
| 3. Should we add the Renkubot users to existing projects when this is deployed for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do that if using the Project Access Tokens we already have allow doing so.
|
should change the folder, resource access control is already 011 |
rokroskar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @olevski for writing this up - I think it's good, with the obvious exception of now all the projects being accessible through a single user.
| KG integration is not active. It could figure this out for public Gitlab projects | ||
| but it would be impossible to find private projects that are not in the KG. | ||
| - Creating a project on Renku will have to include adding this Renkubot user to the project | ||
| with `Reporter` permissions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the same - that it would be part of the KG activation flow.
| - Enabling KG integration on projects that do not have it will have to be modified to also | ||
| add the user to the project similarly to creating a project. | ||
| - Disabling / removing KG integration results in removal of the project from the KG and removing | ||
| the Renkubot user from the project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine the renkubot user could remove itself from a project... ?
|
|
||
| ## Drawbacks | ||
|
|
||
| 1. The Renkubot user will have read access to many repos. If the credentials are stolen or leaked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the major reason not to implement this RFC imho. The bot user will only have read access to the repos, but that might already be too big of a potential risk. AFAICT, it's not possible to create "internal" users like the project bots that gitlab creates automatically when a project access token is created (ref).
| then they could be used to read the contents of private repos. | ||
|
|
||
| 2. When this is rolled out we would automatically add the Renkubot user to all projects. This may | ||
| be confusing to users. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All project maintainers will probably be notified that the bot user has been added. We will need to communicate this clearly to the users so that no one goes and removes it in response.
As @wesjdj suggested.
This idea is definitely worth exploring in a bit more detail. And we have been dismissing it perhaps a bit too early before.