fmorg-git commented on code in PR #11305:
URL: https://github.com/apache/ozone/pull/11305#discussion_r4089541403
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSSecurityUtil.java:
##########
@@ -213,23 +217,116 @@ public static void
ensureResolvedStsFieldsInvariants(OzoneManagerProtocolProtos.
if (!hasSessionToken) {
// If sessionToken is missing/empty, resolved fields must be empty.
- if (s3Auth.hasResolvedStsSessionPolicy() ||
s3Auth.hasResolvedStsRoleArn() ||
- s3Auth.hasResolvedStsOriginalAccessKeyId() ||
s3Auth.hasResolvedStsTempAccessKeyId() ||
- s3Auth.hasResolvedStsSecretKeyId()) {
+ if (hasAnyResolvedStsField(s3Auth)) {
throw new OMException("Resolved STS fields must be empty when
sessionToken is not present", INVALID_TOKEN);
}
return;
}
- ensureResolvedFieldsArePresent(s3Auth);
+ if (!hasAllResolvedStsFields(s3Auth)) {
+ throw new OMException("Resolved STS fields must be present when
sessionToken is present", INVALID_TOKEN);
+ }
+ }
+
+ private static boolean hasAnyResolvedStsField(S3Authentication s3Auth) {
+ return s3Auth.hasResolvedStsSessionPolicy() ||
s3Auth.hasResolvedStsRoleArn() ||
+ s3Auth.hasResolvedStsOriginalAccessKeyId() ||
s3Auth.hasResolvedStsTempAccessKeyId() ||
+ s3Auth.hasResolvedStsSecretKeyId() ||
s3Auth.hasResolvedStsAssumedRoleId() ||
+ s3Auth.hasResolvedStsAssumedRoleUserArn();
}
- private static void ensureResolvedFieldsArePresent(S3Authentication s3Auth)
throws OMException {
- if (!s3Auth.hasResolvedStsSessionPolicy() ||
!s3Auth.hasResolvedStsRoleArn() ||
- !s3Auth.hasResolvedStsOriginalAccessKeyId() ||
!s3Auth.hasResolvedStsTempAccessKeyId() ||
- !s3Auth.hasResolvedStsSecretKeyId()) {
- throw new OMException("Resolved STS fields must be present when
sessionToken is present", INVALID_TOKEN);
+ private static boolean hasAllResolvedStsFields(S3Authentication s3Auth) {
+ return s3Auth.hasResolvedStsSessionPolicy() &&
s3Auth.hasResolvedStsRoleArn() &&
+ s3Auth.hasResolvedStsOriginalAccessKeyId() &&
s3Auth.hasResolvedStsTempAccessKeyId() &&
+ s3Auth.hasResolvedStsSecretKeyId() &&
s3Auth.hasResolvedStsAssumedRoleId() &&
+ s3Auth.hasResolvedStsAssumedRoleUserArn();
+ }
+
+ /**
+ * Copies the STS state that {@link S3SecurityUtil#validateS3Credential} put
in the {@link OzoneManager}
+ * thread local onto {@code s3Auth}, or clears the resolved fields when the
request has no session token.
+ *
+ * <p>The resolved fields are the only STS state visible to the Ratis apply
thread, because the thread locals
+ * belong to the RPC handler thread and do not follow the request onto a
Ratis thread. Resolving here, where
+ * the token has just been verified, is what lets the apply thread rebuild
the request context without
+ * repeating any crypto.</p>
+ *
+ * <p>Returns {@code s3Auth} itself when there is nothing to resolve and
nothing to clear.</p>
+ *
+ * @throws OMException if a session token is present but the token
identifier is not, which means the
+ * request reached this point without passing STS validation
+ */
+ public static S3Authentication resolveS3Authentication(S3Authentication
s3Auth, OzoneManager ozoneManager)
+ throws OMException {
+ final boolean hasSessionToken = s3Auth.hasSessionToken() &&
!s3Auth.getSessionToken().isEmpty();
+ final STSTokenIdentifier stsTokenIdentifier =
OzoneManager.getStsTokenIdentifier();
+
+ // This should not happen, so explicitly throw an error. An existing
sessionToken
+ // implies prior STS validation must have populated the ThreadLocal.
+ if (ozoneManager.isSecurityEnabled() && hasSessionToken &&
stsTokenIdentifier == null) {
+ throw new OMException(
+ "S3Authentication has session token but no STS token identifier in
OzoneManager ThreadLocal",
+ INVALID_REQUEST);
+ }
+
+ if (!hasSessionToken || stsTokenIdentifier == null) {
Review Comment:
yes, this is intentional. The condition was inverted when refactoring from
`OMClientRequest.resolveS3Authentication` into
`STSSecurityUtil.resolveS3Authentication()`. The old positive branch was
`hasSessionToken && stsTokenIdentifier != null;` The new early-return is the
inverted equivalent `!hasSessionToken || stsTokenIdentifier == null`. The
behavior should be unchanged - it populate sresolved fields only when both
session token and ThreadLocal identifier are present, otherwise clear any stale
resolved fields.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
