theforeman / theforeman/puppet-pulpcore
Remove session auth & add IsAdminUser to Pulp settings
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 2
- Forks
- 36
- Avg merge
- 4h 23m
- Merged PRs (30d)
- 1
Description
Today, the Pulpcore auth & permission settings look like:
AUTHENTICATION_BACKENDS = ['pulpcore.app.authentication.PulpNoCreateRemoteUserBackend']
REST_FRAMEWORK__DEFAULT_AUTHENTICATION_CLASSES = (
'rest_framework.authentication.SessionAuthentication',
'pulpcore.app.authentication.PulpRemoteUserAuthentication'
)
However, I don't believe Katello makes use of session cookies talking to Pulp. As such, @mdellweg recommends that we remove its use to improve performance and security.
At the same time, Katello only uses the "built in" admin in Pulp. No user gets created in the database. So, it would likely make more sense to enforce that the user needs to be admin to talk to Pulp.
I recommend that the settings change to look like:
# changed
AUTHENTICATION_BACKENDS = ["pulpcore.app.authentication.PulpNoCreateRemoteUserBackend"]
# same
REST_FRAMEWORK__DEFAULT_AUTHENTICATION_CLASSES = (
'pulpcore.app.authentication.PulpRemoteUserAuthentication',
)
# new option
REST_FRAMEWORK__DEFAULT_PERMISSION_CLASSES = [
'rest_framework.permissions.IsAdminUser'
]
I've tested this on Katello/Satellite running Pulpcore 3.63 and 3.73. I tested some more complex workflows like container push and smart proxy syncing. Everything seems to be working as normal.
The question I can't answer is why we ever had rest_framework.authentication.SessionAuthentication in the first place.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the Pulpcore settings containing AUTHENTICATION_BACKENDS and REST_FRAMEWORK__DEFAULT_AUTHENTICATION_CLASSES. Review why SessionAuthentication is present, then verify the proposed authentication and permission settings against the container push and smart proxy syncing workflows mentioned in the issue. Done means those workflows still work with session authentication removed and IsAdminUser enforced.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api, authentication, authorization
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100