Skip to content

Commit

Permalink
Skip subject login if already authenticated and adds logout method to…
Browse files Browse the repository at this point in the history
… Subject

Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
DarshitChanpura committed Dec 7, 2022
1 parent 0e91f95 commit 5d449c0
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 30 deletions.
11 changes: 11 additions & 0 deletions sandbox/libs/authn/src/main/java/org/opensearch/authn/Subject.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,15 @@ public interface Subject {
* throws SubjectDisabled
*/
void login(final AuthenticationToken token);

/**
* Logs this subject out and kills any session associated with it
*/
void logout();

/**
* Checks the current authentication status of this subject
* @return true if authenticated, false otherwise
*/
boolean isAuthenticated();
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.security.Principal;
import java.util.Objects;

import org.apache.shiro.session.Session;
import org.opensearch.authn.AuthenticationTokenHandler;
import org.opensearch.authn.tokens.AuthenticationToken;
import org.opensearch.authn.Subject;
Expand Down Expand Up @@ -67,4 +68,35 @@ public void login(AuthenticationToken authenticationToken) {
// Login via shiro realm.
shiroSubject.login(authToken);
}

/**
* Logs out this subject
*
* TODO: test this method
*/
@Override
public void logout() {
try {
if (shiroSubject == null) return;
shiroSubject.logout();
// Get current session and kill it before proceeding to create a new session
// TODO: need to study the impact of this
Session session = shiroSubject.getSession(false);
if (session == null) return;
session.stop();
} catch (Exception e) {
// Ignore all errors, as we're trying to silently kill the session
}
}

/**
* A flag to indicate whether this subject is already authenticated
*
* @return true if authenticated, false otherwise
*/
@Override
public boolean isAuthenticated() {
return shiroSubject.isAuthenticated();
}

}
14 changes: 11 additions & 3 deletions server/src/main/java/org/opensearch/identity/noop/NoopSubject.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,19 @@ public String toString() {
return "NoopSubject(principal=" + getPrincipal() + ")";
}

/**
* Logs the user in
*/
@Override
public void login(AuthenticationToken authenticationToken) {
// Do nothing as noop subject is always logged in
}

@Override
public void logout() {
// Do nothing as noop subject should not be logged out
}

@Override
public boolean isAuthenticated() {
// Noop subject is always authenticated
return true;
}
}
30 changes: 3 additions & 27 deletions server/src/main/java/org/opensearch/rest/RestController.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.authc.AuthenticationException;
import org.apache.shiro.session.Session;
import org.opensearch.OpenSearchException;
import org.opensearch.authn.tokens.AuthenticationToken;
import org.opensearch.authn.tokens.BasicAuthToken;
Expand All @@ -64,7 +62,6 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.Serializable;
import java.net.URI;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -680,32 +677,11 @@ private AuthenticationToken tokenType(String authHeader) {
* @param headerToken Attempt login using this token
*/
private static void getShiroSessionAndLogin(Subject subject, AuthenticationToken headerToken) {
// Get or create a new session for this subject when there is none to ensure passing tests.
// TODO: Ensure that shiro session is allowed to be created everytime
logoutCurrentSubjectAndClearSessionIfAny();
// TODO: potential repercussions. i.e. How to handle this in password change situations
// TODO: should logout be implemented to handle this
if (subject.isAuthenticated()) return;

subject.login(headerToken);
}

/**
* Logs out current user and kills the session if any to prevent Shiro from throwing
* {@link org.apache.shiro.session.UnknownSessionException} when calling
* {@link org.apache.shiro.session.mgt.DefaultSessionManager#retrieveSessionFromDataSource(Serializable sessionId)}
*
*/
private static void logoutCurrentSubjectAndClearSessionIfAny() {
try {
// logout current subject
org.apache.shiro.subject.Subject subject = SecurityUtils.getSubject();
if (subject == null) return;
subject.logout();
// Get current session and kill it before proceeding to create a new session
// TODO: need to study the impact of this
Session session = subject.getSession(false);
if (session == null) return;
session.stop();
} catch (Exception e) {
// Ignore all errors, as we're trying to silently kill the session
}
}
}

0 comments on commit 5d449c0

Please sign in to comment.