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.

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

never set REMOTE_USER to the value of SSL_CLIENT_S_DN_CN #360

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

evgeni
Copy link
Member

@evgeni evgeni commented Oct 8, 2024

We only deploy a single user in Pulp: admin
And we do not give out certs with CN=admin, so there is no point in trying to obtain the REMOTE_USER from the CN.

@evgeni evgeni force-pushed the no-dn-user branch 2 times, most recently from fe43bd0 to 0f8aa21 Compare October 8, 2024 07:57
spec/acceptance/basic_spec.rb Outdated Show resolved Hide resolved
@evgeni evgeni force-pushed the no-dn-user branch 2 times, most recently from aa3490e to b2d793a Compare October 8, 2024 08:34
@evgeni
Copy link
Member Author

evgeni commented Oct 8, 2024

Do I want to know why this fails everywhere but CS9?!

@ehelms
Copy link
Member

ehelms commented Oct 8, 2024

And we do not give out certs with CN=admin, so there is no point in trying to obtain the REMOTE_USER from the CN.

We used to, and when we introduced the current method we had to keep the old configuration around for compatibility due to N-1 Capsules.

@evgeni
Copy link
Member Author

evgeni commented Oct 15, 2024

And we do not give out certs with CN=admin, so there is no point in trying to obtain the REMOTE_USER from the CN.

We used to, and when we introduced the current method we had to keep the old configuration around for compatibility due to N-1 Capsules.

But wasn't this for Pulp2 stuff only? At least that's how decipher https://projects.theforeman.org/issues/35004.

Anyway. Old enough to be dropped.

@ehelms
Copy link
Member

ehelms commented Oct 15, 2024

Anyway. Old enough to be dropped.

^ this

@evgeni evgeni force-pushed the no-dn-user branch 2 times, most recently from 261ab13 to 2cba4e9 Compare October 16, 2024 13:57
spec/setup_acceptance_node.pp Outdated Show resolved Hide resolved
@evgeni evgeni force-pushed the no-dn-user branch 3 times, most recently from 5bc30b7 to 7b5f0c2 Compare October 17, 2024 06:12
spec/setup_acceptance_node.pp Outdated Show resolved Hide resolved
@evgeni evgeni force-pushed the no-dn-user branch 2 times, most recently from 03769ef to ddfa49f Compare October 17, 2024 07:29
Comment on lines -3 to +4
pulpcore::apache_https_cert: '/etc/pulpcore-certs/ca-cert.pem'
pulpcore::apache_https_key: '/etc/pulpcore-certs/ca-key.pem'
pulpcore::apache_https_cert: '/etc/pulpcore-certs/client-cert.pem'
pulpcore::apache_https_key: '/etc/pulpcore-certs/client-key.pem'
Copy link
Member Author

Choose a reason for hiding this comment

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

neither is truly right, but I was too lazy to crate a server-*.pem.

@evgeni
Copy link
Member Author

evgeni commented Oct 17, 2024

If y'all think it's beneficial, I could split out the setup changes into an own PR (and add a dedicated server-cert), and then this would/could be reduced to the original "drop the CN=admin handling"?

We only deploy a single user in Pulp: admin
And we do not give out certs with CN=admin, so there is no point in
trying to obtain the REMOTE_USER from the CN.
@evgeni evgeni merged commit 2f80c04 into theforeman:master Oct 21, 2024
23 checks passed
@evgeni evgeni deleted the no-dn-user branch October 21, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants