You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/06/01 09:33:07 UTC
[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1035: JAMES-3757 IMAP/SMTP OIDC extensions should support impersonation
quantranhong1999 commented on code in PR #1035:
URL: https://github.com/apache/james-project/pull/1035#discussion_r886589823
##########
protocols/api/src/main/java/org/apache/james/protocols/api/OIDCSASLParser.java:
##########
@@ -61,11 +69,25 @@ public static Optional<OIDCInitialResponse> parse(String initialResponse) {
String decodeValueWithoutDanglingPart = decodeResult.filter(value -> value.startsWith("n,"))
.map(value -> value.substring(2))
.orElse(decodeResult.get());
- List<String> parts = Splitter.on(SASL_SEPARATOR).splitToList(decodeValueWithoutDanglingPart);
- ImmutableList<String> tokenPart = parts.stream().filter(part -> part.startsWith("auth=")).collect(ImmutableList.toImmutableList());
- ImmutableList<String> userPart = parts.stream().filter(part -> part.startsWith("user=")).collect(ImmutableList.toImmutableList());
- if (tokenPart.size() == 1 && userPart.size() == 1) {
- return Optional.of(new OIDCInitialResponse(userPart.get(0).substring(5), tokenPart.get(0).substring(5).replace(PREFIX_TOKEN, "")));
+
+ StringTokenizer stringTokenizer = new StringTokenizer(decodeValueWithoutDanglingPart, String.valueOf(SASL_SEPARATOR));
+ String tokenPart = null;
+ String userPart = null;
+ String associatedUserPart = null;
+
+ while (stringTokenizer.hasMoreTokens()) {
+ String stringToken = stringTokenizer.nextToken();
+ if (stringToken.startsWith("auth=")) {
+ tokenPart = stringToken.substring(5).replace(PREFIX_TOKEN, "");
+ } else if (stringToken.startsWith("user=")) {
+ userPart = stringToken.substring(5);
+ } else if (stringToken.startsWith("authzid=")) {
+ associatedUserPart = stringToken.substring(8);
Review Comment:
Could we variablize these substring indexes to make them more readable? E.g `ASSOCIATED_USER_VALUE_INDEX = 8`
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java:
##########
@@ -86,37 +91,38 @@ protected void doAuth(AuthenticationAttempt authenticationAttempt, ImapSession s
}
}
- protected void doAuthWithDelegation(AuthenticationAttempt authenticationAttempt, ImapSession session, ImapRequest request, Responder responder, HumanReadableText failed) {
+ protected void doAuthWithDelegation(AuthenticationAttempt authenticationAttempt, ImapSession session, ImapRequest request, Responder responder) {
Preconditions.checkArgument(authenticationAttempt.isDelegation());
Username givenUser = authenticationAttempt.getAuthenticationId();
- Username otherUser = authenticationAttempt.getDelegateUserName().get();
+ if (givenUser == null) {
+ manageFailureCount(session, request, responder);
+ return;
+ }
+ Username otherUser = authenticationAttempt.getDelegateUserName().orElseThrow();
+ doAuthWithDelegation(() -> getMailboxManager().loginAsOtherUser(
+ givenUser,
+ authenticationAttempt.getPassword(),
+ otherUser),
+ session,
+ request, responder);
+ }
+
+ protected void doAuthWithDelegation(MailboxSessionAuthWithDelegationSupplier mailboxSessionSupplier,
+ ImapSession session, ImapRequest request, Responder responder) {
try {
- boolean authFailure = false;
- if (givenUser == null) {
- authFailure = true;
- }
- if (!authFailure) {
- final MailboxManager mailboxManager = getMailboxManager();
- try {
- final MailboxSession mailboxSession = mailboxManager.loginAsOtherUser(givenUser,
- authenticationAttempt.getPassword(),
- otherUser);
- session.authenticated();
- session.setMailboxSession(mailboxSession);
- provisionInbox(session, mailboxManager, mailboxSession);
- okComplete(request, responder);
- } catch (BadCredentialsException e) {
- authFailure = true;
- }
- }
- if (authFailure) {
- manageFailureCount(session, request, responder, failed);
- }
+ final MailboxManager mailboxManager = getMailboxManager();
+ final MailboxSession mailboxSession = mailboxSessionSupplier.get();
+ session.authenticated();
+ session.setMailboxSession(mailboxSession);
+ okComplete(request, responder);
+ provisionInbox(session, mailboxManager, mailboxSession);
+ } catch (BadCredentialsException e) {
+ manageFailureCount(session, request, responder);
} catch (UserDoesNotExistException e) {
- LOGGER.info("User {} does not exist", otherUser, e);
+ LOGGER.info("User does not exist", e);
no(request, responder, HumanReadableText.USER_DOES_NOT_EXIST);
} catch (ForbiddenDelegationException e) {
- LOGGER.info("User {} is not delegated by {}", givenUser, otherUser, e);
+ LOGGER.info("Delegate forbidden", e);
Review Comment:
Why do you reduce the information here?
##########
mailbox/api/src/main/java/org/apache/james/mailbox/exception/ForbiddenDelegationException.java:
##########
@@ -19,5 +19,24 @@
package org.apache.james.mailbox.exception;
+import org.apache.james.core.Username;
+
public class ForbiddenDelegationException extends MailboxException {
+
+ Username givenUser;
+ Username delegateUser;
+
+ public ForbiddenDelegationException(Username givenUser, Username delegateUser) {
+ super("User " + givenUser.asString() + " is not delegated by " + delegateUser.asString());
+ this.givenUser = givenUser;
+ this.delegateUser = delegateUser;
+ }
+
+ public Username givenUser() {
+ return givenUser;
+ }
+
+ public Username delegateUser() {
+ return delegateUser;
+ }
Review Comment:
Where is this be used? I do not see one yet. Maybe you want to use it for tests in the next commits?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org