Changes to Cattle for SAML integration #2085

prachidamle · 2016-10-13T23:42:40Z

Changes include:

Added two instances of GenericWhitelistedProxy - first one will run before APIAuthenticator to allow some configured paths to be proxied through unauthenticated. The second instance will run after APIAuthenticator and perform default checks after auth is passed.
Added AuthSchemaAdditionsPostProcessor to include externalIdTypes (from cattle-global.properties) github_user/github_group/github_team/shibboleth_user/shibboleth_group in
schema
Change to separate allowedIdentities using provider's separator instead of always separating with a comma
Changes to call reload API for rancher-auth-service, when auth config changes

prachidamle · 2016-10-14T02:11:20Z

test failures are unrelated to auth changes here.

test_volume_create_from_driver
test_volume_create_from_user
test_virtual_machine_with_public_ip

Also a jooq error:
Uncaught exception org.jooq.exception.DataChangedException: Database record has been changed
at org.jooq.impl.UpdatableRecordImpl.checkIfChanged(UpdatableRecordImpl.java:550) ~[jooq- at org.jooq.impl.UpdatableRecordImpl.storeUpdate0(UpdatableRecordImpl.java:291)
at org.jooq.impl.UpdatableRecordImpl.access$200(UpdatableRecordImpl.java:90)
at org.jooq.impl.UpdatableRecordImpl$3.operate(UpdatableRecordImpl.java:260)
at org.jooq.impl.RecordDelegate.operate(RecordDelegate.java:123) ~[jooq-3.3.0.jar:na]
at org.jooq.impl.UpdatableRecordImpl.storeUpdate(UpdatableRecordImpl.java:255)
at org.jooq.impl.UpdatableRecordImpl.update(UpdatableRecordImpl.java:149)
at io.cattle.platform.object.impl.JooqObjectManager.persistRecord(JooqObjectManager.java:223)
at io.cattle.platform.object.impl.JooqObjectManager.setFieldsInternal(JooqObjectManager.java:130) ~
at io.cattle.platform.object.impl.JooqObjectManager$3.execute(JooqObjectManager.java:118) ~[cattle-framework-object-0.5.0-SNAPSHOT.jar:na]
at io.cattle.platform.engine.idempotent.Idempotent.change(Idempotent.java:88) ~[cattle-framework-engine-0.5.0-SNAPSHOT.jar:na]
at io.cattle.platform.object.impl.JooqObjectManager.setFields(JooqObjectManager.java:115) ~[cattle-framework-object-0.5.0-SNAPSHOT.jar:na]
at io.cattle.platform.object.impl.JooqObjectManager.setFields(JooqObjectManager.java:110) ~[cattle-framework-object-0.5.0-SNAPSHOT.jar:na]
at io.cattle.platform.object.impl.AbstractObjectManager.setFields(AbstractObjectManager.java:135) ~[cattle-framework-object-0.5.0-SNAPSHOT.jar:na]
at io.cattle.platform.systemstack.listener.SystemStackUpdate.process(SystemStackUpdate.java:117) ~[cattle-system-stack-0.5.0-SNAPSHOT.jar:na]

ibuildthecloud · 2016-10-14T02:24:03Z

...logic/src/main/java/io/cattle/platform/iaas/api/request/handler/GenericWhitelistedProxy.java

+        return allowedPaths;
+    }
+
+    @Inject


Don't need @Inject here

ibuildthecloud · 2016-10-14T02:26:25Z

...packaging/app-config/src/main/resources/META-INF/cattle/iaas-api/spring-iaas-api-context.xml


-    <bean class="io.cattle.platform.iaas.api.request.handler.GenericWhitelistedProxy" />
+    <bean id="AuthenticatedProxy" class="io.cattle.platform.iaas.api.request.handler.GenericWhitelistedProxy" >
+        <property name="noAuthProxy">


No changes should be needed here. The defaults should suffice

Yep, will correct it

ibuildthecloud · 2016-10-14T02:26:48Z

...packaging/app-config/src/main/resources/META-INF/cattle/iaas-api/spring-iaas-api-context.xml

@@ -519,8 +519,27 @@

    <bean id="TokenAccountLookup" class="io.cattle.platform.iaas.api.auth.integration.internal.rancher.TokenAccountLookup" />

+    <bean id="NoAuthenticationProxy" class="io.cattle.platform.iaas.api.request.handler.GenericWhitelistedProxy" >
+        <property name="noAuthProxy">


You can just do value="true", much shorter XML

Yeah then its not a boolean property. It becomes a string. But I will change it and convert string to boolean in setter.

ibuildthecloud · 2016-10-14T02:28:35Z

code/implementation/docker/machine/pom.xml

@@ -39,5 +39,10 @@
            <scope>provided</scope>
            <optional>true</optional>
        </dependency>
+        <dependency>


What code changed in the docker/machine project that needed this? The github diff doesn't show me full filenames

The AuthServiceLauncher is importing some Auth related DynamicStringProperties (for getReloadSettings()) from io.cattle.platform.iaas.api.auth.SecurityConstants and io.cattle.platform.iaas.api.auth.integration.external.ServiceAuthConstants;

ibuildthecloud · 2016-10-14T02:31:13Z

...hema/src/main/java/io/cattle/platform/schema/processor/AuthSchemaAdditionsPostProcessor.java

+public class AuthSchemaAdditionsPostProcessor extends AbstractSchemaPostProcessor implements SchemaPostProcessor, Priority {
+
+    private static final DynamicStringProperty AUTH_SERVICE_EXTERNAL_ID_TYPES = ArchaiusUtil.getString("auth.service.external.id.types");
+    private static final List<String> EXTERNAL_ID_TYPES = new ArrayList<String>();


Don't do this, it defeats the purpose of Dynamic properties. Use DynamicStringListProperty

ibuildthecloud · 2016-10-14T02:31:42Z

...hema/src/main/java/io/cattle/platform/schema/processor/AuthSchemaAdditionsPostProcessor.java

+
+    @Override
+    public SchemaImpl postProcess(SchemaImpl schema, SchemaFactory factory) {
+


Don't put newlines at the beginning of functions

ibuildthecloud · 2016-10-14T02:35:37Z

...logic/src/main/java/io/cattle/platform/iaas/api/request/handler/GenericWhitelistedProxy.java

@@ -214,6 +217,21 @@ protected void generate(final ApiRequest request) throws IOException {
            throw new ClientVisibleException(ResponseCodes.FORBIDDEN);
        }

+        boolean matchesAllowedPath = false;
+        if(isNoAuthProxy()) {
+            if (uri.getPath() != null && allowedPaths != null) {


You are preforming this check on the redirect URL (the url we will hit) not the request URL. You need to check the request URL.

@ibuildthecloud the request URL has the prefix /v1/proxy, while the redirect URL is the actual URL requested by the UI.

example:
requestURL is = http://localhost:8080/v1/proxy/http://localhost:8090/v1-auth/saml/login

redirect is= http://localhost:8090/v1-auth/saml/login

So I am checking the redirect.

Change to add AuthSchemaAdditionsPostProcessor to include github_user/github_group/github_team/shibboleth_user/shibboleth_group in schema Change to separate allowedIdentities using provider's separator Changes to call reload API for rancher-auth-service, when auth config changes Review changes

prachidamle · 2016-10-16T19:59:27Z

@ibuildthecloud please review the changes

prachidamle · 2016-10-16T21:44:30Z

All Test failures are unrelated:

2016-10-16 20:05:45,367 ERROR [3d832d9d-1757-4b15-aee5-c04130f76dd6:3793] [instance:220] [instance.start->(InstanceStart)] [] [cutorService-15] [i.c.p.process.instance.InstanceStart] Failed to Scheduling for instance [220]
866s
1813
2016-10-16 20:05:47,923 ERROR [:] [] [] [] [cutorService-43] [o.a.c.m.context.NoExceptionRunnable ] Uncaught exception java.lang.IllegalStateException: java.lang.IllegalStateException: Failed to download [pyagent]

ibuildthecloud reviewed Oct 14, 2016

View reviewed changes

prachidamle force-pushed the go_auth_saml_integration branch from 47e7a2f to fe873b9 Compare October 16, 2016 19:58

ibuildthecloud merged commit 88724c2 into rancher:master Oct 17, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to Cattle for SAML integration #2085

Changes to Cattle for SAML integration #2085

prachidamle commented Oct 13, 2016

prachidamle commented Oct 14, 2016 •

edited

ibuildthecloud Oct 14, 2016

prachidamle Oct 16, 2016

ibuildthecloud Oct 14, 2016

prachidamle Oct 16, 2016

ibuildthecloud Oct 14, 2016

prachidamle Oct 14, 2016

ibuildthecloud Oct 14, 2016

prachidamle Oct 16, 2016

ibuildthecloud Oct 14, 2016

ibuildthecloud Oct 14, 2016

ibuildthecloud Oct 14, 2016

prachidamle Oct 16, 2016

prachidamle commented Oct 16, 2016

prachidamle commented Oct 16, 2016


		@Override
		public SchemaImpl postProcess(SchemaImpl schema, SchemaFactory factory) {

Changes to Cattle for SAML integration #2085

Changes to Cattle for SAML integration #2085

Conversation

prachidamle commented Oct 13, 2016

prachidamle commented Oct 14, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prachidamle commented Oct 16, 2016

prachidamle commented Oct 16, 2016

prachidamle commented Oct 14, 2016 •

edited