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

Expanded auth system #160

Merged
merged 6 commits into from
Feb 5, 2015
Merged

Expanded auth system #160

merged 6 commits into from
Feb 5, 2015

Conversation

aaannz
Copy link
Contributor

@aaannz aaannz commented Feb 3, 2015

Contrary to the branch name this does not contain nonblocking openid. Instead this introduces plugin based authentication system together with plugins for OpenID, iChain and Fake auth (for development purposes).

my ($self) = @_;
my $headers = $self->req->headers;
my $username = 'FakeUser';
my $email = 'fake@user.org';
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using "Admin" or "Demo"? The reason I came up with fake auth was demoing openQA in front of "customers". I fear "FakeUser" looks a bit odd in such situation :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

@lnussel
Copy link
Contributor

lnussel commented Feb 3, 2015

Are you still storing the full openid url for a user? That is needed in case the openid provider is switched.

@aaannz
Copy link
Contributor Author

aaannz commented Feb 3, 2015

No, I don't store openid url anymore, just username.
I was thinking about provider switch, but I don't think that is properly supported anyway. When you switch provider and compare urls, you'll end up with limited account without admins and operators, since Users.pm:94 won't kick in.
But I agree this might end in dangerous situation when not done carefully. I plan to document that only in documentation though. For our purposes I wanted to make iChain and OpenID switchable without need to recreate users and permissions.

- rename Users table column openid to username
- only when not logged in
- when logged in return 403 as before
- plugins are stored in OpenQA/Auth directory as OpenQA::Auth packages
- plugins must export auth_config, auth_login, auth_logout methods
- plugins return hash (error => int, redirect => string) or undef
  error = 0 means success, if redirect is set, Session controller redirects
- which plugin to use is specified in [auth] section of openqa.ini (by default OpenID)
- when response from auth server is required (as in OpenID), response route is linked to
  auth_response
@aaannz
Copy link
Contributor Author

aaannz commented Feb 4, 2015

I restored storing of openid identity urls in username column and removed any attempts to automatically match username to identity urls. In docu I thus plan to add note if admin wants to change auth method, she should prepare users db migration by herself.

@coolo
Copy link
Contributor

coolo commented Feb 5, 2015

I'd tend to merge this (especially as I need to rebase my branch once more for one more migration), but I think what we're missing is documentation on how to set this up.

@aaannz
Copy link
Contributor Author

aaannz commented Feb 5, 2015

I'll add the proper docu today, but in few words: For OpenID auth there is no change needed. To activate different auth methods just change [auth] method option in openqa.ini.

coolo added a commit that referenced this pull request Feb 5, 2015
@coolo coolo merged commit 77f84e6 into os-autoinst:master Feb 5, 2015
@coolo
Copy link
Contributor

coolo commented Feb 5, 2015

but fake auth is the one we should default to in git IMO.

@aaannz aaannz deleted the nonblocking-openid branch February 5, 2015 09:53
@nilxam
Copy link
Member

nilxam commented Feb 6, 2015

I've a small concern, removed openid section from defaults, actually the code below will ignore your openid config in the openqa.ini
my $v = $cfg && $cfg->val($section, $k);

for example it will always httpsonly even I sets httpsonly=0 in openqa.ini...doesn't it?

@nilxam
Copy link
Member

nilxam commented Feb 6, 2015

hmm..I've opened #166
that is just what I did on my machine, not so sure is it good or bad for your authentication changes/plan, but we can discuss... :)

okurz added a commit to okurz/openQA that referenced this pull request Mar 21, 2020
6b6002d added iChain, in github PR
os-autoinst#160
and progress ticket https://progress.opensuse.org/issues/1729. When
trying to cover authentication modules with better tests I realized that
there is already a typo in one of the request parameters. I doubt iChain
authentication ever worked or would still work. I assume it is not used
at all so we are better off removing it from the current code.
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.

4 participants