theforeman / theforeman/puppet-pulpcore

Remove session auth & add IsAdminUser to Pulp settings

Open
#378 5 comments 1 reaction 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.