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

✨ (api) Allow multiple authentication methods (oidc & basic) #473

Merged
merged 1 commit into from
Nov 15, 2023
Merged

✨ (api) Allow multiple authentication methods (oidc & basic) #473

merged 1 commit into from
Nov 15, 2023

Conversation

Leobouloc
Copy link
Contributor

@Leobouloc Leobouloc commented Oct 18, 2023 •
edited
Loading

Purpose

Currently, an instance of Ralph LRS has a fixed auth method (basic auth and OIDC). The suggestion is to allow multiple auth methods. An example use case could be: "an LMS writes to Ralph LRS via basic Auth; while data analysts fetch from Ralph using OIDC".

Proposal

It is proposed to change RALPH_RUNSERVER_AUTH_BACKEND to RALPH_RUNSERVER_AUTH_BACKENDS and to make it accept comma separated lists as values RALPH_RUNSERVER_AUTH_BACKENDS=basic,oidc.

The auth method for incoming requests will be chosen based on header information (basic, oidc)

  • implement suggestion
  • test that auth methods work individually and simultaneously

@Leobouloc Leobouloc mentioned this pull request Oct 18, 2023
Closed
2 tasks
@Leobouloc Leobouloc added the WIP label Oct 18, 2023
@Leobouloc Leobouloc mentioned this pull request Oct 19, 2023
Closed
2 tasks
@wilbrdt wilbrdt linked an issue Oct 19, 2023 that may be closed by this pull request
Agent field is not valid openId IFI #467
Closed
2 tasks
@Leobouloc Leobouloc force-pushed the feature/api/enforce-scopes-after-unification branch 2 times, most recently from bd6fcb6 to df62058 Compare October 24, 2023 15:16
@Leobouloc Leobouloc changed the base branch from feature/api/enforce-scopes-after-unification to master October 24, 2023 15:18
@Leobouloc Leobouloc force-pushed the feature/api/allow-multiple-auth-after-unification branch 2 times, most recently from 07477f3 to bb2c356 Compare October 24, 2023 16:43
@Leobouloc Leobouloc changed the title (api) Allow multiple authentication methods (oidc & basic) [after #463] (api) Allow multiple authentication methods (oidc & basic) Oct 25, 2023
@Leobouloc Leobouloc force-pushed the feature/api/allow-multiple-auth-after-unification branch from 0e23d4e to a4802c9 Compare October 31, 2023 15:08
@wilbrdt wilbrdt linked an issue Nov 2, 2023 that may be closed by this pull request
[API] Accept simultaneous Basic and OIDC auth methods #445
Closed
@wilbrdt wilbrdt self-requested a review November 2, 2023 09:16
@wilbrdt wilbrdt added this to the 4.0 milestone Nov 2, 2023
wilbrdt
wilbrdt reviewed Nov 2, 2023
View reviewed changes
Copy link
Contributor

@wilbrdt wilbrdt left a comment

Choose a reason for hiding this comment

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

Alt text

CHANGELOG.md Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
src/ralph/api/__init__.py Outdated Show resolved Hide resolved
src/ralph/api/auth/__init__.py Outdated Show resolved Hide resolved
src/ralph/api/auth/__init__.py Outdated Show resolved Hide resolved
src/ralph/api/auth/__init__.py Outdated Show resolved Hide resolved
src/ralph/api/auth/oidc.py Outdated Show resolved Hide resolved
src/ralph/api/auth/oidc.py Outdated Show resolved Hide resolved
src/ralph/conf.py Outdated Show resolved Hide resolved
tests/test_conf.py Outdated Show resolved Hide resolved
jmaupetit
jmaupetit reviewed Nov 7, 2023
View reviewed changes
Copy link
Contributor

@jmaupetit jmaupetit left a comment

Choose a reason for hiding this comment

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

.env.dist Outdated Show resolved Hide resolved
src/ralph/conf.py Outdated Show resolved Hide resolved
src/ralph/conf.py Outdated Show resolved Hide resolved
src/ralph/api/auth/basic.py Outdated Show resolved Hide resolved
src/ralph/api/auth/basic.py Outdated Show resolved Hide resolved
src/ralph/api/auth/basic.py Outdated Show resolved Hide resolved
src/ralph/api/auth/oidc.py Outdated Show resolved Hide resolved
SergioSim
SergioSim reviewed Nov 8, 2023
View reviewed changes
Copy link
Collaborator

@SergioSim SergioSim left a comment

Choose a reason for hiding this comment

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

Looks great 👍 Have only minor questions/suggestions)

image

.env.dist Outdated Show resolved Hide resolved
src/ralph/conf.py Outdated Show resolved Hide resolved
src/ralph/conf.py Outdated Show resolved Hide resolved
src/ralph/conf.py Outdated Show resolved Hide resolved
src/ralph/api/auth/basic.py Outdated Show resolved Hide resolved
src/ralph/api/auth/oidc.py Show resolved Hide resolved
src/ralph/conf.py Outdated Show resolved Hide resolved
src/ralph/api/auth/oidc.py Outdated Show resolved Hide resolved
src/ralph/api/auth/__init__.py Outdated Show resolved Hide resolved
@Leobouloc Leobouloc changed the title (api) Allow multiple authentication methods (oidc & basic) ✨ (api) Allow multiple authentication methods (oidc & basic) Nov 10, 2023
@Leobouloc Leobouloc force-pushed the feature/api/allow-multiple-auth-after-unification branch 3 times, most recently from 0433c93 to c7d8068 Compare November 14, 2023 16:54
@Leobouloc Leobouloc force-pushed the feature/api/allow-multiple-auth-after-unification branch 2 times, most recently from e0195b6 to d49c8b8 Compare November 14, 2023 17:38
Leobouloc
Leobouloc commented Nov 14, 2023
View reviewed changes
src/ralph/conf.py Show resolved Hide resolved
tests/test_conf.py Show resolved Hide resolved
tests/api/auth/test_oidc.py Outdated Show resolved Hide resolved
tests/api/auth/test_basic.py Outdated Show resolved Hide resolved
@Leobouloc Leobouloc force-pushed the feature/api/allow-multiple-auth-after-unification branch 2 times, most recently from 21c7129 to 9d8d6eb Compare November 14, 2023 18:37
wilbrdt
wilbrdt reviewed Nov 15, 2023
View reviewed changes
src/ralph/api/auth/__init__.py Show resolved Hide resolved
src/ralph/api/auth/oidc.py Show resolved Hide resolved
tests/api/auth/test_basic.py Outdated Show resolved Hide resolved
wilbrdt
wilbrdt approved these changes Nov 15, 2023
View reviewed changes
Copy link
Contributor

@wilbrdt wilbrdt left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

@Leobouloc
✨(api) allow multiple authentication backends simultaneously
e1889d2 
Previously, Ralph allowed Basic or OIDC authentication, but not simultaneously.
This PR allows to ralph to handle both at once, answer a use case where machine
users connect through Basic auth, while human users use OIDC (for example).
@Leobouloc Leobouloc force-pushed the feature/api/allow-multiple-auth-after-unification branch from 1fe8b22 to e1889d2 Compare November 15, 2023 15:49
@Leobouloc Leobouloc enabled auto-merge (rebase) November 15, 2023 15:54
@Leobouloc Leobouloc merged commit f44d39e into master Nov 15, 2023
22 checks passed
@Leobouloc Leobouloc deleted the feature/api/allow-multiple-auth-after-unification branch November 15, 2023 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SergioSim SergioSim SergioSim left review comments

@jmaupetit jmaupetit jmaupetit left review comments

@wilbrdt wilbrdt wilbrdt approved these changes

Assignees

@Leobouloc Leobouloc

Labels
needs review
Projects
Ralph roadmap
Status: Done
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

Agent field is not valid openId IFI [API] Accept simultaneous Basic and OIDC auth methods
4 participants
@Leobouloc @jmaupetit @wilbrdt @SergioSim