Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disable admin user API access when enableApplicationLocalLogin is false #505

Merged
merged 1 commit into from May 15, 2020
Merged

Disable admin user API access when enableApplicationLocalLogin is false #505

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 18 additions & 13 deletions manageiq-operator/pkg/helpers/miq-components/httpd_conf.go
View file
Open in desktop
Expand Up @@ -63,19 +63,19 @@ Options SymLinksIfOwnerMatch
func httpdAuthenticationConf(spec *miqv1alpha1.ManageIQSpec) string {
switch spec.HttpdAuthenticationType {
case "openid-connect":
return httpdOIDCAuthConf(spec.OIDCProviderURL, spec.OIDCOAuthIntrospectionURL, spec.ApplicationDomain)
return httpdOIDCAuthConf(spec.OIDCProviderURL, spec.OIDCOAuthIntrospectionURL, spec.ApplicationDomain, *spec.EnableApplicationLocalLogin)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but would it make more sense to just pass the spec object in it's entirety and then the methods pull out what they need?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe when we need one more parameter. Originally this method only needed one or two so it made sense to pass them individually. I feel like 5 is my tipping point

case "external":
return httpdExternalAuthConf()
return httpdExternalAuthConf(*spec.EnableApplicationLocalLogin)
case "active-directory":
return httpdADAuthConf()
return httpdADAuthConf(*spec.EnableApplicationLocalLogin)
case "saml":
return httpdSAMLAuthConf()
default:
return ""
}
}

func httpdExternalAuthConf() string {
func httpdExternalAuthConf(enableLocalLogin bool) string {
s := `
%s

Expand Down Expand Up @@ -111,13 +111,13 @@ func httpdExternalAuthConf() string {
s,
httpdAuthLoadModulesConf(),
httpdAuthLoginFormConf(),
httpdAuthApplicationAPIConf("Basic", "\"External Authentication (httpd) for API\"", apiExtraConfig),
httpdAuthApplicationAPIConf("Basic", "\"External Authentication (httpd) for API\"", apiExtraConfig, enableLocalLogin),
httpdAuthLookupUserDetailsConf(),
httpdAuthRemoteUserConf(),
)
}

func httpdADAuthConf() string {
func httpdADAuthConf(enableLocalLogin bool) string {
s := `
%s

Expand Down Expand Up @@ -153,7 +153,7 @@ func httpdADAuthConf() string {
s,
httpdAuthLoadModulesConf(),
httpdAuthLoginFormConf(),
httpdAuthApplicationAPIConf("Basic", "\"External Authentication (httpd) for API\"", apiExtraConfig),
httpdAuthApplicationAPIConf("Basic", "\"External Authentication (httpd) for API\"", apiExtraConfig, enableLocalLogin),
httpdAuthLookupUserDetailsConf(),
httpdAuthRemoteUserConf(),
)
Expand Down Expand Up @@ -202,7 +202,7 @@ LoadModule auth_mellon_module modules/mod_auth_mellon.so
return fmt.Sprintf(s, httpdAuthRemoteUserConf())
}

func httpdOIDCAuthConf(providerURL, introspectionURL, applicationDomain string) string {
func httpdOIDCAuthConf(providerURL, introspectionURL, applicationDomain string, enableLocalLogin bool) string {
// If these are not provided, we should assume that the user provided a full config
// in a secret, so include the directory for that secret here
if providerURL == "" || introspectionURL == "" || applicationDomain == "" {
Expand Down Expand Up @@ -250,7 +250,7 @@ RequestHeader set X_REMOTE_USER_DOMAIN %%{OIDC_CLAIM_DOMAIN}e env
providerURL,
applicationDomain,
introspectionURL,
httpdAuthApplicationAPIConf("oauth20", "\"External Authentication (oauth20) for API\"", ""),
httpdAuthApplicationAPIConf("oauth20", "\"External Authentication (oauth20) for API\"", "", enableLocalLogin),
)
}

Expand All @@ -275,10 +275,9 @@ func httpdAuthLoginFormConf() string {
`
}

func httpdAuthApplicationAPIConf(authType, authName, extraConfig string) string {
func httpdAuthApplicationAPIConf(authType, authName, extraConfig string, enableLocalLogin bool) string {
s := `
<LocationMatch ^/api(?!\/(v[\d\.]+\/)?product_info$)>
SetEnvIf Authorization '^Basic +YWRtaW46' let_admin_in
SetEnvIf X-Auth-Token '^.+$' let_api_token_in
SetEnvIf X-MIQ-Token '^.+$' let_sys_token_in
SetEnvIf X-CSRF-Token '^.+$' let_csrf_token_in
Expand All @@ -287,17 +286,23 @@ func httpdAuthApplicationAPIConf(authType, authName, extraConfig string) string
AuthName %s
Require valid-user
Order Allow,Deny
Allow from env=let_admin_in
Allow from env=let_api_token_in
Allow from env=let_sys_token_in
Allow from env=let_csrf_token_in
Satisfy Any
%s

%s
</LocationMatch>
`
letAdminIn := ""
if enableLocalLogin {
letAdminIn = `
SetEnvIf Authorization '^Basic +YWRtaW46' let_admin_in
Allow from env=let_admin_in`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does order matter with respect to these settings (I know it does for some other settings in the conf). This PR changes the order, and I wonder if that's a problem or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure it doesn't. @jvlcek can confirm though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does order matter with respect to these settings (I know it does for some other settings in the conf). This PR changes the order, and I wonder if that's a problem or not.

Order does not seem to matter. I just successfully tested with the following segment and it worked fine

<LocationMatch ^/api(?!\/(v[\d\.]+\/)?product_info$)>
  SetEnvIf X-Auth-Token  '^.+$'                 let_api_token_in
  SetEnvIf X-MIQ-Token   '^.+$'                 let_sys_token_in
  SetEnvIf X-CSRF-Token  '^.+$'                 let_csrf_token_in

  AuthType     oauth20
  AuthName     "External Authentication (oidc) for API"

  Require   valid-user
  Order          Allow,Deny
  Allow from env=let_admin_in
  Allow from env=let_api_token_in
  SetEnvIf Authorization '^Basic +YWRtaW46'     let_admin_in
  Allow from env=let_sys_token_in
  Allow from env=let_csrf_token_in
  Satisfy Any
</LocationMatch>

}

return fmt.Sprintf(s, authType, authName, extraConfig)
return fmt.Sprintf(s, authType, authName, letAdminIn, extraConfig)
}

func httpdAuthLookupUserDetailsConf() string {
Expand Down