PR#362: openid,login: utilize actual username when logged using email - ipsilon

ipsilon

#362 openid,login: utilize actual username when logged using email

Closed 2 years ago by ttomecek. Opened 2 years ago by ttomecek.

ttomecek/ipsilon fix358 into master

openid,login: utilize actual username when logged using email

Tomas Tomecek • 2 years ago

cfcda81

ipsilon/providers/openid/auth.py

file modified

+5 -2

		`@@ -212,8 +212,11 @@`
		`return self._template('openid/consent_form.html', **context)`

		`def _response(self, request, session):`
		`- user = session.get_user()`
		`- identity_url = self.cfg.identity_url_template % {'username': user.name}`
		+ # we could use `session.get_user().name` here but when authenticated
		`+ # via email, .name corresponds to email instead of an actual username`
		`+ # https://pagure.io/ipsilon/issue/358`
		`+ userattrs = session.get_user_attrs()`
		`+ identity_url = self.cfg.identity_url_template % {'username': userattrs['_username']}`
		`response = request.answer(`
		`True,`
		`identity=identity_url,`

ipsilon/util/user.py

file modified

		`@@ -21,6 +21,7 @@`
		`self._userdata = dict()`
		`else:`
		`self._userdata = self._get_user_data(username)`
		`+ # username can be an email, https://pagure.io/ipsilon/issue/358`
		`self.name = username`

		`def _get_user_data(self, username):`

ttomecek commented 2 years ago

Fixes: https://pagure.io/ipsilon/issue/358

I welcome guidance regarding testing since I'm not entirely sure how ipsilon obtains all the FAS data.

ngompa commented 2 years ago

:thumbsup:

ngompa commented 2 years ago

@simo Could you please take a look too?

simo commented 2 years ago

Is this behavior specific to Fedora infrastructure?

Sounds like this should be a configurable ...

ttomecek commented 2 years ago

I don't mind making it configurable although it seems that current behavior breaks openid endpoint expectations given the client logs an error:

Discovery verification failure for http://ttomecek@redhat.com.id.fedoraproject.org/

so please let me know how should I proceed

simo commented 2 years ago

So the question is whether _username is always available.
Tracing through the code it requires a module to call the auth_successful function to pass in the user name, this is what my grep shows:
ipsilon/login/authfas.py: return self.lm.auth_successful(self.trans,
ipsilon/login/authform.py: return self.lm.auth_successful(self.trans, user.name, 'password')
ipsilon/login/authgssapi.py: return self.lm.auth_successful(trans, self.user.name,
ipsilon/login/authldap.py: return self.lm.auth_successful(self.trans, username, 'password',
ipsilon/login/authpam.py: return self.lm.auth_successful(
ipsilon/login/authtest.py: return self.lm.auth_successful(self.trans,

These are all the login plugins, so it should be guaranteed ?
If we are confident that's the case I actually see no issue with this patch

ttomecek commented 2 years ago

I'm sorry but I'm not familiar with this codebase to make that call. Let me know what should I do here or how can I verify this works properly locally.

ngompa commented 2 years ago

@abompard, does this complement #363?

ttomecek commented 2 years ago

@abompard's solution seems to be more universal. Thank you, Aurélien!

Pull-Request has been closed by ttomecek

2 years ago