Disable admin user API access when enableApplicationLocalLogin is false #505
Conversation
After this change, any request to the API with basic auth (including the admin user name) will be rejected by httpd with a 401 return code. Fixes ManageIQ#501
|
Checked commit carbonin@af19cdb with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint |
| @@ -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) | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
| if enableLocalLogin { | ||
| letAdminIn = ` | ||
| SetEnvIf Authorization '^Basic +YWRtaW46' let_admin_in | ||
| Allow from env=let_admin_in` |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Pretty sure it doesn't. @jvlcek can confirm though
There was a problem hiding this comment.
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>
Disable admin user API access when enableApplicationLocalLogin is false (cherry picked from commit 3e0699e)
|
Jansa backport details: |
After this change, any request to the API with basic auth (including
the admin user name) will be rejected by httpd with a 401 return code.
Fixes #501