Don't display a remote username on /admin/users/edit.php if no remote username exists

Bug #1160093 reported by Kristina Hoeppner
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
High
Robert Lyon

Bug Description

Per default, Mahara displays the "Username for external authentication" on the user account admin page no matter whether the account has actually set a remoteuser value or not. This is confusing when you want to check if an auth instance works correctly because you will have to check the database (in 2 different places) or at least do a user report which pulls the remotuser value from the "correct" DB table.

Mahara should only display a remoteuser if it really exists.

Changed in mahara:
status: Confirmed → In Progress
assignee: nobody → Jonathan Sharp (jonathans)
Revision history for this message
Jonathan Sharp (jonathans) wrote :

change (experimental) pushed

suggested functionality: Mahara should only display a remoteuser value if the selected authplugin (in the Authentication method dropdown) is not '... internal'.

If the Authentication method is external then some javascript displays the 'Username for external authentication' field - which will contain the remote username if set.
Only institutions' for which the user is a member will have their authentication methods shown

The 'Username for external authentication' help has been updated

Revision history for this message
Robert Lyon (robertl-9) wrote :
Aaron Wells (u-aaronw)
Changed in mahara:
assignee: Jonathan Sharp (jonathans) → Robert Lyon (robertl-9)
Revision history for this message
Robert Lyon (robertl-9) wrote :

have reworked the patch to fit better with changes in the admin/user/edit.php that have occurred after initial patch was submitted.

https://reviews.mahara.org/#/c/2191/4

now the remote auth field should only show if an external auth has been selected and update the field with existing remote auth value if exists.

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/2191
Committed: http://gitorious.org/mahara/mahara/commit/f2baf73ac986c92b06337e2e4d06112ced0f6c29
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit f2baf73ac986c92b06337e2e4d06112ced0f6c29
Author: Robert Lyon <email address hidden>
Date: Wed Aug 21 10:26:47 2013 +1200

Bug #1160093: Remote username only shown for external auth method

Signed-off-by: Jonathan Sharp <email address hidden>
Signed-off-by: Robert Lyon <email address hidden>
Change-Id: Ic6b3f89fa0c7873402c0d3f65708b5cc9e335728

Aaron Wells (u-aaronw)
Changed in mahara:
status: In Progress → Fix Committed
Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

I added the tag "nominatedfeature" though it's more like a bug so I can pick it up for the manual and highlight this change.

tags: added: nominatedfeature
Revision history for this message
Aaron Wells (u-aaronw) wrote :

So, while researching some stuff relating to ldapsync, I gained a bit more insight into how the remote username & parent authority work and work together:

1. The only auth plugins that use "remote username" are XMLRPC and SAML (optionally for SAML)

2. These are both services where you *don't* enter your username into the Mahara login form. Instead, you log in via an external service, and then it communicates the username to Mahara during the SSO process.

3. Where "remote username" comes in, is that when a username logs in via one of these processes, it checks for a record in the auth_remote_user table for this authinstance and that username. That record will point to a usr.id value for the Mahara user account that it should authenticate them to.

4. It kinda only makes sense for auth instances where you don't enter the username directly into Mahara. Otherwise, at the login screen, you may find yourself wondering, do I enter my Mahara username, or my LDAP username?

Now, how does parent authority work?

1. Only XMLRPC has a parent authority.

2. If a user has that XMLRPC as their auth instance, they can also log in using the parent authority -- and get to the same Mahara account! And vice versa.

3. In terms of user creation, the first time you roam over from the XMLRPC remote server, if the XMLRPC auth instance has a parent, it will create your account with the parent as your authinstance rather than the XMLRPC.

4. On subsequent logins, it checks auth_remote_user for a user with the matching remote username, and either the XMLRPC or its parent as their auth instance, and it logs you in as that user.

So, based on this a user will need to set up a remote username if:

1. Their auth instance is XMLRPC
2. Their auth instance is parent to an XMLRPC. (And note that any auth type can be parent to XMLRPC -- even internal)
3. Their auth instance is a SAML with the remote username feature enabled.

So, I'm putting together a patch that adds a "needs_remote_username()" method to the Auth class, which will indicate whether a particular auth instance needs a remote username or not. And then we'll display the external username field on the account settings page based on the value returned by that function. I'm also changing the create_user() function so that it automatically checks whether the new user's auth instance needs a remote username and supplies it if so.

Revision history for this message
Aaron Wells (u-aaronw) wrote :
Changed in mahara:
status: Fix Committed → In Progress
Revision history for this message
Aaron Wells (u-aaronw) wrote :

Even with my latest commits, there's still a bug case on the user edit screen:

1. Set up two auth instances that can take an external username
2. Navigate to the account settings page for a user and set their external username with one of these auth instances
3. Now, change their auth setting to the other auth instance that takes an external username
4. Change their external username (for that second auth instance) to something else
5. Save.
6. Now, go back to the user's account settings page, and change their auth instance back to the first auth instance.

Expected Result: It should show the external username you entered for that first auth instance
Actual Result: It will show the external username you entered for the second auth instance

What we'd need to do, in order to make this work properly, is pull the external usernames for all your auth instances, store them in Javascript, and pop them in and out as you switch amongst auth instances. The squishy part, is what if you've edited the external username field, then you switch to another auth instance, then switch back, all without saving? Should Javascript retain your (changed but not saved) username and bring it back? Or should it just toss out your unsaved changed external username when you switch auth instances? I kinda favor the latter, just because it's easier and therefore less error-prone, and this is a corner-case anyway.

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Never mind... on further testing it actually does work, and does give me the expected result! :) It was actually already written to look up and store your external username in all the different auth instances, store them in JS, and swap them in and out as you switch auth instances.

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Ah, it was sorta buggy, if you did this:

1. Set up two auth instances that can take an external username
2. Assign a user to one of them, and give them an external username in that auth.
3. Save.
4. Now, go to the user's account settings page, and switch them to the other external auth

Expected Result: External username field on the page should be empty, because the user doesn't have an external username registered with this auth yet
Actual Result: The external username field on the page is NOT empty, but continues to show the external username for the first auth.

That's because the code didn't bother switching the field to blank, if there was no username stored.

It's handy to have it populate the stored username from the author auth automatically. But I think it's handier to tell users the truth, and only show them data in that form field if there actually is corresponding data in the auth_remote_user table. So I'll add this to my patch also.

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2452
Committed: http://gitorious.org/mahara/mahara/commit/20512fdb0c485e651fab76dbfc7c2268a30913c7
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 20512fdb0c485e651fab76dbfc7c2268a30913c7
Author: Aaron Wells <email address hidden>
Date: Mon Aug 26 17:00:38 2013 +1200

Changing PluginAuth API to specifically indicate whether Auth requires remote username

Bug 1160093: This adds a few new methods to the Auth class, which represents an auth instance:

- is_parent_authority(): Indicates whether this auth instance is a parent authority or not
- get_parent_authority(): Gets the ID of this auth instance's parent authority
- needs_remote_username(): Indicates whether this auth instance needs the user to have a
remote username setting (in auth_remote_user table)

I've also updated the SAML and XMLRPC auth types, which are the only ones that use remote username.
And I've updated create_user() to automatically populate auth_remote_user() for auth
instances that use it.

Note that an auth instance of ANY type will need a remote username if it's the parent to another
authority (the parent feature allows a user to log in via the parent or the child auth instance;
so it's quite possible for the user to have different usernames in the two of them. Currently
only XMLRPC uses the parent auth feature.)

Lastly, also updated the documentation of LiveUser->create_user() to indicate that it only
uses the $remoteauth parameter as a boolean (which was true even before my code changes).

Change-Id: I39b1b74e68cdbc9c2632b886655caaaece1bd312

Aaron Wells (u-aaronw)
Changed in mahara:
status: In Progress → Fix Committed
Aaron Wells (u-aaronw)
Changed in mahara:
milestone: 1.8rc1 → 1.8.0
Aaron Wells (u-aaronw)
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.