Limit access to bridgehead configuration repository #4
Conversation
This change enforces that secret sync clients can only request project access tokens for the bridgehead configuration repository of the respective site.
central/src/gitlab.rs
Outdated
| /// Derive the bridgehead configuration repository from the beam id of the requester. | ||
| /// Example return value: "bridgehead-configurations/bridgehead-config-berlin" | ||
| fn derive_bridgehead_config_repo_from_beam_id(requester: &AppId) -> Result<String, String> { | ||
| let mut parts = requester.as_ref().splitn(3, '.'); | ||
| let (_app, proxy, broker) = (parts.next().unwrap(), parts.next().unwrap(), parts.next().unwrap()); | ||
|
|
||
| let group = match broker { | ||
| "broker.ccp-it.dktk.dkfz.de" => "bridgehead-configurations", // ccp | ||
| "broker.bbmri.sample.de" => "bbmri-bridgehead-configs", // bbmri | ||
| "broker.bbmri.de" => "bbmri-bridgehead-configs", // gbn | ||
| "test-no-real-data.broker.samply.de" => "bridgehead-configurations", // cce, itcc, kr | ||
| "broker.hector.dkfz.de" => "dhki", // dhki | ||
| _ => return Err(format!("Bridgehead configuration repository not known for beam id {requester}")), | ||
| }; | ||
|
|
||
| Ok(match group { | ||
| "bridgehead-configurations" => format!("{group}/bridgehead-config-{proxy}"), | ||
| "dhki" => format!("{group}/{proxy}-bk"), | ||
| _ => format!("{group}/{proxy}"), | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
@torbrenner I implemented the list you sent me; please confirm this is correct. I found one more irregularity: projects in the bridgehead-configurations group need the bridgehead-config- prefix, whereas projects in the bbmri-bridgehead-configs group don't. I can't check the other groups because I don't have permissions to see them on GitLab.
central/src/gitlab.rs
Outdated
| let group = match broker { | ||
| "broker.ccp-it.dktk.dkfz.de" => "bridgehead-configurations", // ccp | ||
| "broker.bbmri.sample.de" => "bbmri-bridgehead-configs", // bbmri | ||
| "broker.bbmri.de" => "bbmri-bridgehead-configs", // gbn | ||
| "test-no-real-data.broker.samply.de" => "bridgehead-configurations", // cce, itcc, kr | ||
| "broker.hector.dkfz.de" => "dhki", // dhki | ||
| _ => return Err(format!("Bridgehead configuration repository not known for beam id {requester}")), | ||
| }; |
There was a problem hiding this comment.
Not sure what you discussed with Torben and Martin but from my side I think I would have made this a config option instead of deriving it from the broker id but that seems fine as well
There was a problem hiding this comment.
I assume you imply there is one central secret sync per broker? It would make sense then, yes.
There was a problem hiding this comment.
Yeah there pretty much has to be because we have to actively pull tasks from the broker and we really ever poll from one broker. But leave it like that it will be fine
Co-authored-by: Jan <[email protected]>
Threated
deleted the
pr-limit-access-to-bridgehead-configuration-repo
branch
| let mut parts = requester.as_ref().splitn(3, '.'); | ||
| let (_app, proxy, broker) = (parts.next().unwrap(), parts.next().unwrap(), parts.next().unwrap()); |
There was a problem hiding this comment.
@timakro Sorry for being late, but do you have to unwrap? Isn't this available as a function of the AppId?
There was a problem hiding this comment.
AppId is missing a function to get the broker id. Should I open a PR in the beam repo to add this?
| let group = match broker { | ||
| "broker.ccp-it.dktk.dkfz.de" => "bridgehead-configurations", // ccp | ||
| "broker.bbmri.sample.de" => "bbmri-bridgehead-configs", // bbmri | ||
| "broker.bbmri.de" => "bbmri-bridgehead-configs", // gbn | ||
| "test-no-real-data.broker.samply.de" => "bridgehead-configurations", // cce, itcc, kr | ||
| "broker.hector.dkfz.de" => "dhki", // dhki | ||
| _ => return Err(format!("Bridgehead configuration repository group not known for broker {broker}")), | ||
| }; | ||
|
|
||
| Ok(match group { | ||
| "bridgehead-configurations" => format!("{group}/bridgehead-config-{proxy}"), | ||
| "dhki" => format!("{group}/{proxy}-bk"), | ||
| _ => format!("{group}/{proxy}"), | ||
| }) |
There was a problem hiding this comment.
Future work: Would it be possible to externalize this to e.g. a config file?
There was a problem hiding this comment.
To get rid of the first match we could use Jans idea and make it a config option for the secret sync central component. If I understand correctly, each broker has their own secret sync central component so that should work fine.
For the second match I could add a config option where you can specify an optional prefix for the repository name.
This change enforces that secret sync clients can only request project access tokens for the bridgehead configuration repository of the respective site.