spolavarpau1 commented on code in PR #894:
URL: https://github.com/apache/ranger/pull/894#discussion_r3087907286
##########
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSecurityContextFormationFilter.java:
##########
@@ -174,7 +174,7 @@ private int getAuthType(Authentication auth,
HttpServletRequest request) {
Object ssoEnabledObj = request.getAttribute("ssoEnabled");
boolean ssoEnabled = ssoEnabledObj != null ?
Boolean.parseBoolean(String.valueOf(ssoEnabledObj)) :
PropertiesUtil.getBooleanProperty("ranger.sso.enabled", false);
-
+ String authMethod =
PropertiesUtil.getProperty("ranger.authentication.method", "NONE");
Review Comment:
Why are we explicitly setting NONE?
##########
security-admin/src/main/webapp/react-webapp/src/views/SideBar/SideBarBody.jsx:
##########
@@ -220,21 +220,34 @@ export const SideBarBody = (props) => {
if (checkKnoxSSOresp?.status == "419") {
setUserProfile(null);
window.location.replace("login.jsp");
+ } else {
+ handleLogout(null);
}
console.error(`Error occurred while logout! ${error}`);
}
};
const handleLogout = async (checkKnoxSSOVal) => {
try {
+ // For SAML, we must do a full browser navigation to /logout so Spring
Security
+ // can perform the SLO redirect chain (Ranger -> IdP -> back to Ranger).
+ // An AJAX call cannot follow the cross-domain SLO redirect, which
causes the
+ // IdP session to remain active and results in an automatic re-login
loop.
+ const currentUserProfile = getUserProfile();
+ const authMethod =
currentUserProfile?.configProperties?.authenticationMethod;
+ if (authMethod?.toUpperCase() === "SAML") {
+ window.location.replace("logout");
+ return;
+ }
+
await fetchApi({
url: "logout",
baseURL: "",
headers: {
"cache-control": "no-cache"
}
});
- if (checkKnoxSSOVal !== undefined || checkKnoxSSOVal !== null) {
+ if (checkKnoxSSOVal !== undefined && checkKnoxSSOVal !== null) {
Review Comment:
Can you check if this has any regression impact when KnoxSSO is enabled?
##########
security-admin/src/main/resources/conf.dist/ranger-admin-site.xml:
##########
@@ -453,4 +453,46 @@
<name>xasecure.audit.jaas.Client.option.principal</name>
<value></value>
</property>
+ <property>
+ <name>ranger.saml.attribute.username</name>
+ <value>NameID</value>
+ </property>
+ <property>
+ <name>ranger.saml.attribute.role</name>
+ <value>groups</value>
+ </property>
+ <property>
+ <name>ranger.saml.default.role</name>
+ <value>ROLE_USER</value>
+ </property>
+ <property>
+ <name>ranger.saml.admin.group</name>
+ <value>ranger-admins</value>
+ </property>
+ <!-- SAML SP / IdP file paths and identity -->
Review Comment:
Can you check if we can move the SAML related properties to
ranger-admin-default-site.xml
##########
security-admin/src/main/java/org/apache/ranger/rest/UserREST.java:
##########
@@ -274,6 +274,7 @@ public VXPortalUser getUserProfile(@Context
HttpServletRequest request) {
long inactivityTimeout =
PropertiesUtil.getLongProperty("ranger.service.inactivity.timeout", 15 * 60);
configProperties.put("inactivityTimeout",
Long.toString(inactivityTimeout));
+ configProperties.put("authenticationMethod",
PropertiesUtil.getProperty("ranger.authentication.method", "NONE"));
Review Comment:
I am not completely sure why we had to add this?
##########
security-admin/src/main/resources/conf.dist/ranger-admin-site.xml:
##########
@@ -453,4 +453,46 @@
<name>xasecure.audit.jaas.Client.option.principal</name>
<value></value>
</property>
+ <property>
Review Comment:
Do we need to specify these properties in our distribution of config file as
we already have default values in our code?
--
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]
