Add signed batch machine run context / ADR#1772
Conversation
krowvin
changed the title
MikeNeilson
left a comment
MikeNeilson
left a comment
There was a problem hiding this comment.
I'm being a bit pushy here. AND I may in fact be wrong about what we can accomplish with Keycloak and thus may end up having to go this route (there's nothing fundmentally wrong with the design... other that too much reliance on properties, though that's an initial design problem not a new problem) but I think if we can get Keycloak to handle the load the transition is a lot simpler.
| final String email = claims.get(EMAIL_CLAIM, String.class); | ||
| return dao.createUser(preferredUserName, oidcPrincipal, givenName, email); | ||
| DataApiPrincipal dataApiPrincipal = dao.createUser(preferredUserName, oidcPrincipal, givenName, email); | ||
| BatchJobContext.prepareContext(ctx, dataApiPrincipal); |
There was a problem hiding this comment.
If the "batch" user is getting created randomly here, we have an issue, In the context of a batch process this should be a failure.
| public static final String REQUESTED_BY_ATTR = "BatchRequestedBy"; | ||
| public static final String DISPATCH_SOURCE_ATTR = "BatchDispatchSource"; | ||
|
|
||
| public static final String SECRET_PROPERTY = "cwms.dataapi.batch.jobContext.secret"; |
There was a problem hiding this comment.
why is Batch becoming an issuer of secrets?
My understanding is that Keycloak would provide the JWT and CDA would just consume it.
| if (username == null) { | ||
| return false; | ||
| } | ||
| String machineUsers = readSetting(MACHINE_USERS_PROPERTY, DEFAULT_MACHINE_USERS); |
There was a problem hiding this comment.
CDA should not need to know anything about this, the JWT provided by Keycloak can embed a claim of "machine-auth" or something and decisions made from that.
| throw new CwmsAuthException("Batch job context missing run_as_office", | ||
| HttpServletResponse.SC_UNAUTHORIZED); | ||
| } | ||
| ctx.attribute(RUN_AS_OFFICE_ATTR, office.toUpperCase(Locale.ROOT)); |
There was a problem hiding this comment.
While appropriate to put in to a future logging context... these shouldn't need to be part of the Request/Response attributes. Downstream components needing to know that is a definite code smell.
|
|
||
| Provide CWMS Data API with a trusted batch run context for jobs that execute through a shared machine identity. | ||
|
|
||
| Batch runtimes will authenticate to CDA with a service account (via Keycloak). Each job will also provide a signed context token that identifies the authorized job launch context, including the office for which the scheduler or API approved the run. |
There was a problem hiding this comment.
Setting up additional signing is going to be difficult to get right, and will involve yet-more CDK changes. Granted they won't be difficult.
I would like us to first determine if we can, in some way, setup keycloak to be able to receive the required information (such as the office, and specific job identification) and return it back in the already signed JWT access token.
Some of the other information can, and should, still be provided but it doesn't really required singing it's just informational. The office would be used to set the session context (or by the future authorization system) to appropriately limit operations.
e.g. office + job identification can be readily tied to a policy to appropriately limit operations..
2 participants
Summary
Discussed adding M2M auth for use with CWMS Batch Jobs. This is the first step towards enabling this form of authentication for use in CWBI.
Adds CDA support for signed batch job run context when requests are made by configured batch machine users.
This allows batch runtimes to authenticate with a shared Keycloak service account while still giving CDA trusted job launch context from the dispatcher, such as the authorized run office, job id, requester, and dispatch source.
Also adds ADR 0009 documenting the signed batch machine run context design.
Related Issue
Closes #
Validation
JDK 21
./gradlew.bat :cwms-data-api:test --tests cwms.cda.security.BatchJobContextTest./gradlew.bat :cwms-data-api:testRatingsVerticalDatumExtractorTest; newBatchJobContextTestpassed.Checklist