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

WIP: Adds ability to use API Key to "sign in" #708

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eyJhb
Copy link

@eyJhb eyJhb commented Jan 3, 2025

Description (Proposed Changes)

  • Adds a API Key to the sign in screen

Link to the issue :

Tests

Unsure how to do this.

Checklist

Please check that the PR fulfills all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • Set a 100 character limit in your editor/IDE to avoid white space diffs in the PR
  • Tests for the changes have been added (for bug fixes / features)
  • Added yourself to AUTHORS.md
  • Updated/added relevant documentation (doc comments with ///).
  • Added relevant reviewers.

image

@Dieterbe
Copy link
Contributor

Dieterbe commented Jan 3, 2025

Why do we want this?

@eyJhb
Copy link
Author

eyJhb commented Jan 3, 2025

Why do we want this?

Look at the linked issue here wger-project/wger#1847 , without this they wouldn't be able to use the app at all. Since the idea is to set up a 3rd party auth service in front, e.g. Authelia, which ensures that the user is authenticated and passes it on to Wger.

This in turn means, that the user does not have a username/password combination that works for Wger (only a username), and they would be unable to sign in using the app. Instead they can provide the API token from the settings page, and use that instead.

@Dieterbe
Copy link
Contributor

Dieterbe commented Jan 3, 2025

interesting. thanks. so i suppose this would also require a way to register users with wger and instantiate api keys for them somehow.

@eyJhb
Copy link
Author

eyJhb commented Jan 3, 2025

That's taken care of in this PR wger-project/wger#1859 , when enabled, Wger should assume that any request with e.g. Remote-User: eyjhb in the header, is authenticated and will then create the user.

So in the case for the app, that wouldn't be required/make any sense to implement the part for registering, as that would be outside the scope of the app, in the case of having that specific setup. The only way to implement it, would be to open a webview of the instance URL, let the user authenticate, and then point them to the API key page. But I think that is out of scope. I think it's expected, that with a setup that uses Auth Proxy, that registering using the app is not possible :)

@rolandgeider
Copy link
Member

Yes this would be used when authenticating via an external service or SSO

@eyJhb
Copy link
Author

eyJhb commented Jan 3, 2025

But it would maybe be nice, to hide it under an "advanced options", together with custom server URL as well.

@rolandgeider
Copy link
Member

But it would maybe be nice, to hide it under an "advanced options", together with custom server URL as well.

yes, definitely. I can help you with that (and with the tests)

@eyJhb
Copy link
Author

eyJhb commented Jan 3, 2025

I might have time to look at it, but feel free to take over the PR. I just made it to get the ball rolling, and show the basic idea that I have :)

EDIT: changed title and converted to draft to reflect this. I should have started by doing that.

@eyJhb eyJhb changed the title Adds ability to use API Key to "sign in" WIP: Adds ability to use API Key to "sign in" Jan 3, 2025
@eyJhb eyJhb marked this pull request as draft January 3, 2025 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants