Skip to content

LCORE-283: Introduce authentication to LS-stack API #189

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
Merged
merged 2 commits into from
Jul 8, 2025
Merged

LCORE-283: Introduce authentication to LS-stack API #189

merged 2 commits into from
Jul 8, 2025

Conversation

@umago
Copy link
Contributor

@umago umago commented Jul 4, 2025

Description

This patch introduce authentication mechanisms to the Lightspeed-stack API.

Following what is in road-core/service, all 3 methods have been reintroduced:

  • noop - Which is just a No-op method, used for development
  • noop-with-token - Similar to No-op but require a Authorization Header to be passed as part of the request. For development use only
  • k8s - Which authenticates with Kubernetes/OCP

A few notes about this patch:

  1. noop is the default authentication method. In road-core/service the default method was "k8s" but, since this is not a "k8s" first project
  2. The "skip_user_id_check" was removed from the AuthInterface return values because it wasn't used anywhere
  3. The "k8s.py" file contains a lot of pyright warnings, 58 to be exact. This file was basically c&p from the existing one in road-core/service.So I left those to be fixed later. We may want to refactor how we do things there.
  4. No dev_config has been introduced as part of this patch

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #LCORE-283
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Unit-tests with extensive coverage
  • Manually tested

@tisnik tisnik changed the title Introduce authentication to LS-stack API LCORE-283: Introduce authentication to LS-stack API Jul 7, 2025
tisnik
tisnik approved these changes Jul 7, 2025
View reviewed changes
Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

it looks ok-ish. Would be nice to be "clean" on CI, but probably ok for now (I can fix it later I guess)

@manstis PTAL - it will affect you too

@umago
Introduce authentication to LS-stack API
d11c09d 
This patch introduce authentication mechanisms to the Lightspeed-stack
API.

Following what is in road-core/service, all 3 methods have been
reintroduced:
   * noop - Which is just a No-op method, used for development
   * noop-with-token - Similar to No-op but require a Authorization
     Header to be passed as part of the request. For development use
     only
   * k8s - Which authenticates with Kubernetes/OCP

A few notes about this patch:

1. noop is the default authentication method. In road-core/service the
   default method was "k8s" but, since this is not a "k8s" first project
2. The "skip_user_id_check" was removed from the AuthInterface return
   values because it wasn't used anywhere
3. The "k8s.py" file contains a lot of pyright warnings, 58 to be exact.
   This file was basically c&p from the existing one in
   road-core/service.So I left those to be fixed later. We may want to
   refactor how we do things there.
4. No dev_config has been introduced as part of this patch

Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
@umago
Copy link
Contributor Author

umago commented Jul 7, 2025

it looks ok-ish. Would be nice to be "clean" on CI, but probably ok for now (I can fix it later I guess)

@manstis PTAL - it will affect you too

@tisnik if you want I can work on that, what needs to be cleaned up ?

@tisnik
Copy link
Contributor

tisnik commented Jul 7, 2025

it looks ok-ish. Would be nice to be "clean" on CI, but probably ok for now (I can fix it later I guess)
@manstis PTAL - it will affect you too

@tisnik if you want I can work on that, what needs to be cleaned up ?

When I looked at CI, there were some linter failures. Now are gone -> we are ok. thanks

@umago
Copy link
Contributor Author

umago commented Jul 7, 2025 •
edited
Loading

@tisnik ah sorry, it was conflict from other patches that merged before. I fixed it now. I thought it was something to clean in the code itself, my bad

tisnik
tisnik approved these changes Jul 7, 2025
View reviewed changes
Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

nice one!

@tisnik
Copy link
Contributor

tisnik commented Jul 7, 2025

@manstis please take a look at this one. From my perspective it's perfectly ok, but it might affect you

manstis
manstis approved these changes Jul 7, 2025
View reviewed changes
Copy link
Contributor

@manstis manstis left a comment

Choose a reason for hiding this comment

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

Code LGTM 👍

It would be nice to document how the k8s authentication works.

It's not clear (to me) what I might need to do in k8s for this to work!?!

@umago
Fix K8SAuthDependency docstring
4da4893 
Fix the K8SAuthDependency docstring to include information about the
authentication/authorization process for the k8s authentication module.

Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
tisnik
tisnik approved these changes Jul 8, 2025
View reviewed changes
Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

nice one

@tisnik tisnik merged commit 9c1783f into lightspeed-core:main Jul 8, 2025
16 checks passed
omertuc added a commit to omertuc/lightspeed-stack that referenced this pull request Jul 10, 2025
@omertuc
Unpack auth tuple to get token
0dd4e16 
Since d11c09d
(lightspeed-core#189), the
"`auth`" tuple returned by `AuthInterface` contains the user ID, user
name, and token. We want to pass the token to the MCP server, but we
accidentally passed the entire stringified tuple instead, causing the
MCP server to fail to authenticate the request. This commit fixes that
issue by unpacking the `auth` tuple to extract the token before passing
it to `get_response`.
@omertuc omertuc mentioned this pull request Jul 10, 2025
Merged
18 tasks
omertuc added a commit to omertuc/lightspeed-stack that referenced this pull request Jul 10, 2025
@omertuc
Unpack auth tuple to get token
0ebf65e 
Since d11c09d
(lightspeed-core#189), the
"`auth`" tuple returned by `AuthInterface` contains the user ID, user
name, and token. We want to pass the token to the MCP server, but we
accidentally passed the entire stringified tuple instead, causing the
MCP server to fail to authenticate the request. This commit fixes that
issue by unpacking the `auth` tuple to extract the token before passing
it to `get_response`.
omertuc added a commit to omertuc/lightspeed-stack that referenced this pull request Jul 10, 2025
@omertuc
Unpack auth tuple to get token
ed276d5 
Since d11c09d
(lightspeed-core#189), the
"`auth`" tuple returned by `AuthInterface` contains the user ID, user
name, and token. We want to pass the token to the MCP server, but we
accidentally passed the entire stringified tuple instead, causing the
MCP server to fail to authenticate the request. This commit fixes that
issue by unpacking the `auth` tuple to extract the token before passing
it to `get_response`.
omertuc added a commit to omertuc/lightspeed-stack that referenced this pull request Jul 10, 2025
@omertuc
Unpack auth tuple to get token
5bfa6b0 
Since d11c09d
(lightspeed-core#189), the
"`auth`" tuple returned by `AuthInterface` contains the user ID, user
name, and token. We want to pass the token to the MCP server, but we
accidentally passed the entire stringified tuple instead, causing the
MCP server to fail to authenticate the request. This commit fixes that
issue by unpacking the `auth` tuple to extract the token before passing
it to `get_response`.
omertuc added a commit to omertuc/lightspeed-stack that referenced this pull request Jul 11, 2025
@omertuc
Unpack auth tuple to get token
c843505 
Since d11c09d
(lightspeed-core#189), the
"`auth`" tuple returned by `AuthInterface` contains the user ID, user
name, and token. We want to pass the token to the MCP server, but we
accidentally passed the entire stringified tuple instead, causing the
MCP server to fail to authenticate the request. This commit fixes that
issue by unpacking the `auth` tuple to extract the token before passing
it to `get_response`.
omertuc added a commit to omertuc/lightspeed-stack that referenced this pull request Jul 11, 2025
@omertuc
Unpack auth tuple to get token
11e7cf2 
Since d11c09d
(lightspeed-core#189), the
"`auth`" tuple returned by `AuthInterface` contains the user ID, user
name, and token. We want to pass the token to the MCP server, but we
accidentally passed the entire stringified tuple instead, causing the
MCP server to fail to authenticate the request. This commit fixes that
issue by unpacking the `auth` tuple to extract the token before passing
it to `get_response`.
omertuc added a commit to omertuc/lightspeed-stack that referenced this pull request Jul 11, 2025
@omertuc
Unpack auth tuple to get token
b1cb492 
Since d11c09d
(lightspeed-core#189), the
"`auth`" tuple returned by `AuthInterface` contains the user ID, user
name, and token. We want to pass the token to the MCP server, but we
accidentally passed the entire stringified tuple instead, causing the
MCP server to fail to authenticate the request. This commit fixes that
issue by unpacking the `auth` tuple to extract the token before passing
it to `get_response`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@manstis manstis manstis approved these changes

@tisnik tisnik tisnik approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@umago @tisnik @manstis