Skip to content

Require domain rights and service token when registering a database context - #140

Open
mtaghiza wants to merge 1 commit into
mainfrom
feature/rdb-dbcontext-auth
Open

mtaghiza wants to merge 1 commit into
mainfrom
feature/rdb-dbcontext-auth

Conversation

@mtaghiza

@mtaghiza mtaghiza commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Changes:

  1. Authorization: addDbContextToRDBComputeDomain now checks the registerDatabaseContext right on the domain's root context (new JOBMAccessControl.canRegisterDatabaseContext) instead of testing for the admin role by name.

  2. Authentication: the endpoint now requires the X-Service-Auth-ID header and rejects it unless it resolves to the domain in the path, same as submitJob. RACMClient.registerRDBComputeDbContext gained a serviceToken param as well.

Callers now need both the domain's service token and the register right, when only a plain user token used to be enough in the past.

@mtaghiza
mtaghiza requested a review from glemson September 2, 2026 11:38

@glemson glemson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the one change I proposed is better. otherwise this is very similar conceptually to the change I propose in PR #136 interestingly. There the database needed an update, which here is not required.

boolean canEditComputeDomain(User u, ComputeDomain cd) {
return racm.doesUserHaveRoleOnResource(u.getUsername(), cd.getResourceContext().getUuid(), RACMNames.CONTEXT_ROOTRESOURCE_PUBDID, RACMNames.R_COMPUTE_DOMAIN_ROOT_ADMIN);
}
boolean canRegisterDatabaseContext(User u, ComputeDomain cd) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would it be better to add this method on RDBDomainManager?

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.

2 participants