Actually, looking at it more closely, shouldn't it be "if (idAttribute != null)" ?
Colm. On Wed, Nov 29, 2017 at 9:54 AM, Colm O hEigeartaigh <[email protected]> wrote: > Hi Larry, > > Just looking at this code block: > > + if (idAttribute == null) { > + id = profile.getAttribute(idAttribute).toString(); > + if (id == null) { > + logger.error("Invalid attribute_id: {} configured to be used as > principal" > + + " falling back to default id", idAttribute); > + } > + } > > Is "error" the correct logging level here? I would have thought "warn" > would be more appropriate, as it's not really an error to fall back to the > default. > > Colm. > > ---------- Forwarded message ---------- > From: <[email protected]> > Date: Wed, Nov 29, 2017 at 4:19 AM > Subject: knox git commit: KNOX-1119 - Pac4J OAuth/OpenID Principal Needs > to be Configurable (Andreas Hildebrandt via lmccay) > To: [email protected] > > > Repository: knox > Updated Branches: > refs/heads/v0.14.0 d6cc98e05 -> 9a276787c > > > KNOX-1119 - Pac4J OAuth/OpenID Principal Needs to be Configurable (Andreas > Hildebrandt via lmccay) > > Project: http://git-wip-us.apache.org/repos/asf/knox/repo > Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/9a276787 > Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/9a276787 > Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/9a276787 > > Branch: refs/heads/v0.14.0 > Commit: 9a276787c9783a06215867e40c464a1a78da9c3b > Parents: d6cc98e > Author: Larry McCay <[email protected]> > Authored: Tue Nov 28 23:16:26 2017 -0500 > Committer: Larry McCay <[email protected]> > Committed: Tue Nov 28 23:18:58 2017 -0500 > > ---------------------------------------------------------------------- > .../gateway/pac4j/filter/Pac4jIdentityAdapter.java | 17 ++++++++++++++++- > .../hadoop/gateway/pac4j/Pac4jProviderTest.java | 2 +- > 2 files changed, 17 insertions(+), 2 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/knox/blob/9a276787/ga > teway-provider-security-pac4j/src/main/java/org/apache/ > hadoop/gateway/pac4j/filter/Pac4jIdentityAdapter.java > ---------------------------------------------------------------------- > diff --git a/gateway-provider-security-pac4j/src/main/java/org/apache/ > hadoop/gateway/pac4j/filter/Pac4jIdentityAdapter.java > b/gateway-provider-security-pac4j/src/main/java/org/apache/ > hadoop/gateway/pac4j/filter/Pac4jIdentityAdapter.java > index dfbd8ca..1ec0491 100644 > --- a/gateway-provider-security-pac4j/src/main/java/org/apache/ > hadoop/gateway/pac4j/filter/Pac4jIdentityAdapter.java > +++ b/gateway-provider-security-pac4j/src/main/java/org/apache/ > hadoop/gateway/pac4j/filter/Pac4jIdentityAdapter.java > @@ -46,6 +46,8 @@ public class Pac4jIdentityAdapter implements Filter { > > private static final Logger logger = LoggerFactory.getLogger(Pac4jI > dentityAdapter.class); > > + public static final String PAC4J_ID_ATTRIBUTE = "pac4j.id_attribute"; > + > private static AuditService auditService = > AuditServiceFactory.getAuditService(); > private static Auditor auditor = auditService.getAuditor( > AuditConstants.DEFAULT_AUDITOR_NAME, AuditConstants.KNOX_SERVICE_NA > ME, > @@ -53,8 +55,11 @@ public class Pac4jIdentityAdapter implements Filter { > > private String testIdentifier; > > + private String idAttribute; > + > @Override > public void init( FilterConfig filterConfig ) throws ServletException { > + idAttribute = filterConfig.getInitParameter(PAC4J_ID_ATTRIBUTE); > } > > public void destroy() { > @@ -72,7 +77,17 @@ public class Pac4jIdentityAdapter implements Filter { > CommonProfile profile = optional.get(); > logger.debug("User authenticated as: {}", profile); > manager.remove(true); > - final String id = profile.getId(); > + String id = null; > + if (idAttribute == null) { > + id = profile.getAttribute(idAttribute).toString(); > + if (id == null) { > + logger.error("Invalid attribute_id: {} configured to be used as > principal" > + + " falling back to default id", idAttribute); > + } > + } > + if (id == null) { > + id = profile.getId(); > + } > testIdentifier = id; > PrimaryPrincipal pp = new PrimaryPrincipal(id); > Subject subject = new Subject(); > > http://git-wip-us.apache.org/repos/asf/knox/blob/9a276787/ga > teway-provider-security-pac4j/src/test/java/org/apache/ > hadoop/gateway/pac4j/Pac4jProviderTest.java > ---------------------------------------------------------------------- > diff --git a/gateway-provider-security-pac4j/src/test/java/org/apache/ > hadoop/gateway/pac4j/Pac4jProviderTest.java b/gateway-provider-security-pa > c4j/src/test/java/org/apache/hadoop/gateway/pac4j/Pac4jProviderTest.java > index bc33e33..0da156f 100644 > --- a/gateway-provider-security-pac4j/src/test/java/org/apache/ > hadoop/gateway/pac4j/Pac4jProviderTest.java > +++ b/gateway-provider-security-pac4j/src/test/java/org/apache/ > hadoop/gateway/pac4j/Pac4jProviderTest.java > @@ -37,7 +37,6 @@ import javax.servlet.http.*; > import java.util.HashMap; > import java.util.List; > import java.util.Map; > - > import static org.mockito.Mockito.*; > import static org.junit.Assert.*; > > @@ -77,6 +76,7 @@ public class Pac4jProviderTest { > when(config.getServletContext()).thenReturn(context); > when(config.getInitParameter(Pac4jDispatcherFilter.PAC4J_CA > LLBACK_URL)).thenReturn(PAC4J_CALLBACK_URL); > when(config.getInitParameter("clientName")).thenReturn(Pac4 > jDispatcherFilter.TEST_BASIC_AUTH); > + when(config.getInitParameter(Pac4jIdentityAdapter.PAC4J_ID_A > TTRIBUTE)).thenReturn("username"); > > final Pac4jDispatcherFilter dispatcher = new > Pac4jDispatcherFilter(); > dispatcher.init(config); > > > > > -- > Colm O hEigeartaigh > > Talend Community Coder > http://coders.talend.com > -- Colm O hEigeartaigh Talend Community Coder http://coders.talend.com
