Adding external authentication httpd configuration files #210

abellotti · 2017-09-01T22:37:14Z

Introduced HTTPD_AUTH_CONFIGURATION to select which authentication configuration files to include
authentication.conf includes the configuration-* file based on HTTPD_AUTH_CONFIGURATION
common configurations sections are shared by different types minimizing duplication.
Introduced HTTPD_AUTH_KERBEROS_REALMS needed for kerberos authentication

abellotti · 2017-09-01T22:40:00Z

Need to test:

Also need to:

Update README.md to talk about the auth-config entries, auth-configuration and auth-kerberos-realms
Duplicate to the ext-db yaml.

miq-bot · 2017-09-25T14:59:13Z

This pull request is not mergeable. Please rebase and repush.

abellotti · 2017-09-25T19:22:51Z

Kerberos SSO support will require ManageIQ/container-httpd#24

abellotti · 2017-10-06T13:19:08Z

No changes needed here for a while, I'm good with this.

ping @carbonin @bdunne can I borrow your 👀 for review/merge ? Thanks.

carbonin · 2017-10-06T13:47:57Z

README.md

@@ -638,6 +653,8 @@ Excluding the content of the files, a SAML auth-config map data section may look
 apiVersion: v1
 data:
  auth-type: saml
+  auth-configuration: saml


What is the distinction between auth-type and auth-configuration? In what situations will they not be the same value?

Maybe it would be worth making the example one of those cases to avoid confusion?

👍 I've added an auth-type / auth-configuration specification matrix table.

Thanks, the table is really helpful. It allows me to ask better questions 😉 haha

carbonin · 2017-10-06T13:49:39Z

templates/miq-template-ext-db.yaml

        ProxyPreserveHost on
        ProxyPass        /ws/ ws://${NAME}/ws/
        ProxyPassReverse /ws/ ws://${NAME}/ws/
-        ProxyPass        / http://${NAME}/
+
+        # For httpd, some ErrorDocuments must by served by the front-end httpd


I think front-end is overloaded here. I think "httpd pod" would be better to distinguish it from the miq-frontend pod

carbonin · 2017-10-06T13:56:48Z

templates/miq-template-ext-db.yaml

+        # For httpd, some ErrorDocuments must by served by the front-end httpd
+        RewriteCond %{REQUEST_URI} !^/proxy_pages
+
+        # For SAML /saml2 is only served by front-end httpd by mod_auth_mellon


Same as above, we should avoid using "front-end" to reference the auth httpd pod

- Introduced HTTPD_AUTH_CONFIGURATION to select which authentication configuration files to include - authentication.conf includes the configuration-* file based on HTTPD_AUTH_CONFIGURATION - common configurations sections are shared by different types minimizing duplication. - Introduced HTTPD_AUTH_KERBEROS_REALMS needed for kerberos authentication

- With :80 virtualhost, need to tell mod_auth_mellon that IdP access us via https - /saml2 is only server by front-end httpd, do not redirect to back-end httpd.

- Added an auth-type/auth-configuration specification matrix.

carbonin · 2017-10-09T14:07:53Z

README.md

@@ -573,6 +573,22 @@ The config map includes the following:

 	`internal` is the default type, anything else is considered external. `auth-type` could include strings like: ipa, ldap, active_directory, saml or simply custom.

+* The httpd configuration type `auth-configuration`, default is `internal`
+
+	This parameter drive which configuration files httpd will load upon start-up. Supported values are:


Typo

"This parameter drive" -> "This parameter drives"

carbonin · 2017-10-09T14:09:59Z

README.md

+	| active-directory | Active Directory domain realm join
+	| saml | SAML based authentication (Keycloak, etc.)
+
+* The kerberos realms joined `auth-kerberos-realms`, default is `undefined`


"The kerberos realms joined" seems strangely worded to me. Maybe "The kerberos realm to join"?

Also if this is indeed plural, how do you specify multiple? Comma separated?

carbonin · 2017-10-09T14:11:00Z

README.md

@@ -607,7 +623,19 @@ _Examples_:

 Binary files can be specified in the configuration map in their base64 encoded format with a basename having a `.base64` extension. Such files are then converted back to binary as they are copied to their target path.

-When an /etc/sssd/sssd.conf file is included in the configuration map, the httpd pod automatically enables the sssd service upon startup.


Is this no longer true? Why remove this line? I feel like the table doesn't make this point clear.

carbonin · 2017-10-09T14:16:04Z

README.md

+| auth-type | auth-configuration | Note |
+|-----------|--------------------|------|
+| internal  | internal           | Database / ManageIQ Ldap(s) / Amazon    |
+| ldap      | external           |     |


What happens if someone configures a nonsense combination of these parameters (e.g. ldap + active-directory or saml + external)? It seems like the only one with multiple combinations is active-directory so can we assume the auth-configuration value for the others?

Or even better, can we make auth-configuration specific to active-directory? Maybe something like active-directory-config-set or do we intend to add more auth-configuration options for other auth-types in the future?

The auth-configuration is directly drives what Apache configurations we support. they cannot specify anything else. As we add support for others in the future, i.e. openid, we'd be adding Apache config/include files to reflect the new auth-configuration. So the auth-configuration set is strict/concrete. The auth-type identifies the identify provider we're targeting, note how multiple types share the same auth-configuration apache include files. As far as the values there, if we want/need to to differentiate via the auth-config CLI a generated config map for Keycloak vs. let's say Active Directory Federated Services, we can have auth-type (saml | adfs | okta | etc.), but the auth-configuration must be saml.

Hopefully this helps a bit.

Okay, that's fine. So we can't make auth-configuration more specific to AD (even though right now that is the only auth-type that could possibly have more than one valid auth-configuration). But my first question still stands. Are we doing anything to validate against nonsense combinations of auth-type + auth-configuration?

After discussion, it seems like the values in the left column do not have any effect on the runtime of the application.

The values in the right column can be used for the same purpose as the auth-type is currently being used for (deciding whether to copy config files listed in auth-configuration.conf) as well as which specific httpd auth configuration files we should include in the authentication.conf file, so we can remove the values on the left side (but can keep using the existing auth-type name for the field).

carbonin · 2017-10-09T14:16:44Z

README.md

@@ -638,6 +653,8 @@ Excluding the content of the files, a SAML auth-config map data section may look
 apiVersion: v1
 data:
  auth-type: saml
+  auth-configuration: saml


Thanks, the table is really helpful. It allows me to ask better questions 😉 haha

- Brought back text about effect of including sssd.conf

the definition of the auth-config maps where we removed the need for (and confusion) of the auth-configuration parameter. Now simply driven by auth-type (internal, external, active-direectory, saml) and auth-kerberos-realms.

abellotti · 2017-10-10T19:56:28Z

@carbonin simplified and updated. also updated the auth-config CLI and tested end-to-end for IPA. still good. Thanks!!

miq-bot · 2017-10-10T19:58:46Z

Checked commits abellotti/manageiq-pods@bb939d9~...26258c9 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

carbonin · 2017-10-10T19:59:49Z

README.md

+
+	When configuring external authentication against IPA, Active Directory or Ldap, this parameter defines the kerberos realm httpd is configured against, i.e. `example.com`
+
+	When specifying multiple Kerberos realms, they need to be space separated.


miq-bot added the wip label Sep 1, 2017

abellotti mentioned this pull request Sep 6, 2017

Fixed issue where SSSD wasn't started after an auth configmap update. ManageIQ/container-httpd#16

Merged

miq-bot added the unmergeable label Sep 25, 2017

abellotti force-pushed the ext-auth-apache-config branch from 0cec376 to bce85c6 Compare September 25, 2017 20:11

miq-bot removed the unmergeable label Sep 25, 2017

abellotti changed the title ~~[WIP] Adding external authentication httpd configuration files~~ Adding external authentication httpd configuration files Sep 25, 2017

miq-bot removed the wip label Sep 25, 2017

carbonin reviewed Oct 6, 2017

View reviewed changes

abellotti added 6 commits October 6, 2017 22:16

Fixes to the httpd configuration to work with SAML

7e86fe9

- With :80 virtualhost, need to tell mod_auth_mellon that IdP access us via https - /saml2 is only server by front-end httpd, do not redirect to back-end httpd.

Copied the miq-template.yaml changes to miq-template-ext-db.yaml

0d37f7c

Updated README.md to discuss new auth-config map parameters added.

1cd80d7

Added fixes for Kerberos SSO to work.

a62b7e5

- Clarification of the proxy_pages/saml2 comments in the templates.

b8aa658

- Added an auth-type/auth-configuration specification matrix.

abellotti force-pushed the ext-auth-apache-config branch from ba1503c to b8aa658 Compare October 7, 2017 02:53

carbonin reviewed Oct 9, 2017

View reviewed changes

abellotti added 2 commits October 9, 2017 22:35

- Fixed typos and clarified some of the parameters.

dc35688

- Brought back text about effect of including sssd.conf

Updated README.md and templates/miq-template-* to simplify

26258c9

the definition of the auth-config maps where we removed the need for (and confusion) of the auth-configuration parameter. Now simply driven by auth-type (internal, external, active-direectory, saml) and auth-kerberos-realms.

carbonin reviewed Oct 10, 2017

View reviewed changes

carbonin approved these changes Oct 10, 2017

View reviewed changes

carbonin self-assigned this Oct 10, 2017

carbonin added the enhancement label Oct 10, 2017

carbonin merged commit f74c933 into ManageIQ:master Oct 10, 2017

carbonin added this to the Sprint 71 Ending Oct 16, 2017 milestone Oct 10, 2017

abellotti deleted the ext-auth-apache-config branch October 10, 2017 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding external authentication httpd configuration files #210

Adding external authentication httpd configuration files #210

abellotti commented Sep 1, 2017

abellotti commented Sep 1, 2017 •

edited

Loading

miq-bot commented Sep 25, 2017

abellotti commented Sep 25, 2017

abellotti commented Oct 6, 2017

carbonin Oct 6, 2017 •

edited

Loading

abellotti Oct 7, 2017

carbonin Oct 9, 2017

carbonin Oct 6, 2017

carbonin Oct 6, 2017

carbonin Oct 9, 2017

carbonin Oct 9, 2017

carbonin Oct 9, 2017

carbonin Oct 9, 2017

abellotti Oct 10, 2017 •

edited

Loading

carbonin Oct 10, 2017

carbonin Oct 10, 2017

carbonin Oct 9, 2017

abellotti commented Oct 10, 2017

miq-bot commented Oct 10, 2017

carbonin Oct 10, 2017

		@@ -607,7 +623,19 @@ _Examples_:

		Binary files can be specified in the configuration map in their base64 encoded format with a basename having a `.base64` extension. Such files are then converted back to binary as they are copied to their target path.

		When an /etc/sssd/sssd.conf file is included in the configuration map, the httpd pod automatically enables the sssd service upon startup.


		When configuring external authentication against IPA, Active Directory or Ldap, this parameter defines the kerberos realm httpd is configured against, i.e. `example.com`

		When specifying multiple Kerberos realms, they need to be space separated.

Adding external authentication httpd configuration files #210

Adding external authentication httpd configuration files #210

Conversation

abellotti commented Sep 1, 2017

abellotti commented Sep 1, 2017 • edited Loading

miq-bot commented Sep 25, 2017

abellotti commented Sep 25, 2017

abellotti commented Oct 6, 2017

carbonin Oct 6, 2017 • edited Loading

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

abellotti Oct 10, 2017 • edited Loading

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

abellotti commented Oct 10, 2017

miq-bot commented Oct 10, 2017

Choose a reason for hiding this comment

abellotti commented Sep 1, 2017 •

edited

Loading

carbonin Oct 6, 2017 •

edited

Loading

abellotti Oct 10, 2017 •

edited

Loading