Cannot login to web staff client if work station does not exist in database

Bug #1467663 reported by Jason Stephenson on 2015-06-22
44
This bug affects 9 people
Affects Status Importance Assigned to Milestone
Evergreen
Medium
Unassigned

Bug Description

Evergreen Version: 2.7+
OpenSRF version: 2.4 (but doesn't apply)
Postgres version: Doesn't matter

If you have registered a workstation with the web staff client that subsequently disappears from the database, you cannot login via the web staff client until after you have cleared the workstation entries from local storage in the browser. This is not likely to come up in production environments, but could easily arise in training or demo situations where the database is refreshed periodically.

To reproduce this bug,

Login with the web staff client.
Register a new workstation from the Administration menu.
Logout and log back in to see that the registration is working.
Logout of the web staff client.
In PgAdmin or some other database tool, delete the entry from actor.workstation for the workstation that you registered above.
When you next try to login with the web staff client, the login will fail.

The osrfsys.log will have entries like the following for each failed login attempt:
open-ils.auth 2015-06-22 15:40:02 [INFO:30482:oils_event.c:46:1435002001306031]
Creating new event: WORKSTATION_NOT_FOUND

If you clear the workstation information from your browser's local storage for the domain where you are using the web staff client, you will be able to login again, but you will need to register your workstation again.

The above also assumes that you have only registered 1 workstation. If you've registered multiple workstations, you can still login with any that exist in the database.

Bill Erickson (berick) wrote :

The XUL client would remove the workstation from the local config, log the user in, then present the user with the workstation registration interface. The browser client does not require a workstation, so after we clean up the bogus WS, we'll have more options to consider.

The simplest work flow for now might be:

* Login fails with WORKSTATION_NOT_FOUND
* Delete the bogus WS from "eg.workstation.default" and "eg.workstation.all" via Hatch API
* Log the user in without a workstation
* Show a dialog to the user explaining that the workstation does not exist then offer two options: 1) go to the workstation admin page (admin/workstation/index) or 2) continue without a workstation.

Related to this, I've been considering whether we should add a toggle for "this computer requires a registered workstation", similar to the existing "This workstation uses a remote print / storage service" toggle. That would of course change the proposed work flow.

Download full text (3.7 KiB)

On Fri, Jul 3, 2015 at 3:57 PM, Bill Erickson <email address hidden> wrote:

> The XUL client would remove the workstation from the local config, log
> the user in, then present the user with the workstation registration
> interface. The browser client does not require a workstation, so after
> we clean up the bogus WS, we'll have more options to consider.
>
> The simplest work flow for now might be:
>
> * Login fails with WORKSTATION_NOT_FOUND
> * Delete the bogus WS from "eg.workstation.default" and
> "eg.workstation.all" via Hatch API
> * Log the user in without a workstation
> * Show a dialog to the user explaining that the workstation does not exist
> then offer two options: 1) go to the workstation admin page
> (admin/workstation/index) or 2) continue without a workstation.
>
> Related to this, I've been considering whether we should add a toggle
> for "this computer requires a registered workstation", similar to the
> existing "This workstation uses a remote print / storage service"
> toggle. That would of course change the proposed work flow.
>
>
That (and part (2) above) would get a big -1 from me. Trying to use the
web staff client without a workstation registered is only going to get
worse than it is today -- spurious patron-only template includes, bad
default location assumptions, crossing the streams with user settings,
etc. I think requiring registration is the only way forward.

However, we'll need to make local storage (both hatch and in-browser) more
robust, because if you turn on hatch today on a workstation that has been
in use without it, you "lose" all your local settings, because in-browser
settings get ignored if hatch is there and you use the generic get/set
methods.

--miker

> --
> You received this bug notification because you are a member of Evergreen
> Bug Wranglers, which is subscribed to Evergreen.
> https://bugs.launchpad.net/bugs/1467663
>
> Title:
> Cannot login to web staff client if work station does not exist in
> database
>
> Status in Evergreen - Open ILS:
> New
>
> Bug description:
> Evergreen Version: 2.7+
> OpenSRF version: 2.4 (but doesn't apply)
> Postgres version: Doesn't matter
>
> If you have registered a workstation with the web staff client that
> subsequently disappears from the database, you cannot login via the
> web staff client until after you have cleared the workstation entries
> from local storage in the browser. This is not likely to come up in
> production environments, but could easily arise in training or demo
> situations where the database is refreshed periodically.
>
> To reproduce this bug,
>
> Login with the web staff client.
> Register a new workstation from the Administration menu.
> Logout and log back in to see that the registration is working.
> Logout of the web staff client.
> In PgAdmin or some other database tool, delete the entry from
> actor.workstation for the workstation that you registered above.
> When you next try to login with the web staff client, the login will
> fail.
>
> The osrfsys.log will have entries like the following for each failed
> login attempt:
> open-ils.auth 2015-06-22 15:40:02
> [INFO:30482:oils_e...

Read more...

Bill Erickson (berick) wrote :

On Mon, Jul 6, 2015 at 10:28 AM, Mike Rylander <email address hidden> wrote:

> On Fri, Jul 3, 2015 at 3:57 PM, Bill Erickson <email address hidden>
> wrote:
>
> > The XUL client would remove the workstation from the local config, log
> > the user in, then present the user with the workstation registration
> > interface. The browser client does not require a workstation, so after
> > we clean up the bogus WS, we'll have more options to consider.
> >
> > The simplest work flow for now might be:
> >
> > * Login fails with WORKSTATION_NOT_FOUND
> > * Delete the bogus WS from "eg.workstation.default" and
> > "eg.workstation.all" via Hatch API
> > * Log the user in without a workstation
> > * Show a dialog to the user explaining that the workstation does not
> exist
> > then offer two options: 1) go to the workstation admin page
> > (admin/workstation/index) or 2) continue without a workstation.
> >
> > Related to this, I've been considering whether we should add a toggle
> > for "this computer requires a registered workstation", similar to the
> > existing "This workstation uses a remote print / storage service"
> > toggle. That would of course change the proposed work flow.
> >
> >
> That (and part (2) above) would get a big -1 from me. Trying to use the
> web staff client without a workstation registered is only going to get
> worse than it is today -- spurious patron-only template includes, bad
> default location assumptions, crossing the streams with user settings,
> etc. I think requiring registration is the only way forward.
>

Requiring a workstation for everyone would certainly simplify some things.
Until now, there has been a (vague) notion that not every client would
require a workstation. If there is no value in that and/or it's going to
cause chaos, then +1 to killing that idea.

>
> However, we'll need to make local storage (both hatch and in-browser) more
> robust, because if you turn on hatch today on a workstation that has been
> in use without it, you "lose" all your local settings, because in-browser
> settings get ignored if hatch is there and you use the generic get/set
> methods.
>
>
True. I'll create a separate bug for that.

-b

Changed in evergreen:
assignee: nobody → Victoria Lewis (vlewis-q)
Bill Erickson (berick) wrote :

Taking required workstations into consideration, I have some additional thoughts on this.

For starters, we probably should refactor the workstation admin page to function as a standalone dialog so it can be accessed from the login page and from the admin page. This way we're not duplicating code.

For the work flow, I see 3 scenarios:

Scenario 1: No WS is registered (new users).

1. Hide the workstation selector by default, since it's empty.
2. Allow the user to log in w/o workstation.
3. Display the WS admin dialog.
4. Message within the dialog explains that a WS is required.
5. After registration, log out, reload the login page.

This is basically the XUL client work flow for new logins.

Scenario 2: a single invalid WS is registered

1. Remove the invalid workstation from local storage
2. Log the user in w/ the already provided credentials (w/o workstation)
3. Display the WS admin dialog.
4. Display a message to the user saying the WS was invalid and the user needs to register a new WS
5. After registration, log out, reload the login page.

Scenario 3: multiple WS's registered.

1. Remove the invalid workstation from local storage
2. Leave the WS selector visible, so the user can select a different WS.
3. Display a message to the user explaining the WS was invalid, they can either log in w/ a different WS or register a new WS. The message contains a link/button to launch to WS admin dialog.
4. If clicked, Display the WS admin dialog.
5. After registration, log out, reload the login page.

To prevent users from navigating away from the login page (or WS dialog) when no workstation is present, we add a check to the auth service code which redirects to the login page when egAuth.user().wsid() is null. (There's already an if{} block in the code for this).

Changed in evergreen:
assignee: Victoria Lewis (vlewis-q) → nobody
Kathy Lussier (klussier) wrote :

Hi Bill,

The workflows suggested in your three scenarios sound good to me.

For clarity, on the last step where you said "After registration, log out, reload the login page" the system is automatically logging you out and reloading the login page as soon as you click register, correct?

Thanks for looking at this!
Kathy

Bill Erickson (berick) wrote :

Yes, exactly. The system would automatically log you out after registration completes. The user would not have to manually log out.

Kathy Lussier (klussier) wrote :

I'm adding the webstaffprodcirc tag and 2.next target for this bug as something we need to resolve before the web client is ready for production use. My largest concern isn't with the bug Jason originally reported, but with the current ability to use the web client without a registered workstation.

In our testing, we came across several problems that only occurred if the tester forgot to register a workstation. We therefore need to ensure there is a way to enforce workstation registration. Even if those issues were resolved, we would feel must more comfortable requiring workstation registration since we use the workstation quite often when troubleshooting problems.

Since the discussion on this bug was already towards workflow for required workstation registration, I'm adding the tag here instead of opening a separate bug on the issue.

tags: added: webstaffprodcirc
Changed in evergreen:
milestone: none → 2.next
importance: Low → Medium
Changed in evergreen:
status: New → Confirmed
Bill Erickson (berick) wrote :

I encounter this bug frequently when rebuilding databases. It's confusing and annoying. Here's a branch that partially resolves the problem.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1467663-webstaff-workstation-kludge

It does not implement the full work flow described above. It's intended instead as a stop-gap until the fuller work flow can be implemented.

When a user logs in with a workstation that is not registered with the server:

1. Removes the offending workstation from the locally stored list of all workstations
2. Clears the default workstation value if it matches the offending workstation
3. Warns the user that the workstation was not found and indicates the login will proceed without a workstation.
4. Proceeds with login sans workstation.

It's a far cry from the desired work flow, but it at least communicates to the user why nothing is working and allows the login to continue without having to manually edit the local storage values.

Some of this code can be reused for the final / full implementation.

NOTE: if we merge this code, we need to leave the bug open to complete the implementation.

tags: added: pullrequest
removed: bitesize
Bill Erickson (berick) wrote :

Adding 'pullrequest' for partial implementation, removing 'bitesize' cuz it ain't.

Changed in evergreen:
assignee: nobody → Jennifer Pringle (jpringle-u)
Bill Erickson (berick) wrote :

Jennifer, I've started working on a more complete solution to this bug. Removing the pullrequest for the stop-gap code and reassigning to me.

Changed in evergreen:
assignee: Jennifer Pringle (jpringle-u) → Bill Erickson (berick)
tags: removed: pullrequest
Bill Erickson (berick) wrote :

Pushed a new branch implementing the work flow described above, with slight variation.

Instead of a workstation dialog, there is now a dedicated workstation admin page, accessible from the Workstation Administration page. When no workstations exist, the user is allowed to login then directed to the new WS admin page. The same thing occurs when an invalid WS is used, with the additional step of having the WS admin page delete the offending WS in the process. In all cases, if the user is not logged in with a valid WS, they will be sent to the WS admin page, so they can create and use a workstation.

This also repairs the "Use Now" button and implements the workstation "Delete" (now called "Remove") button so that users (with permission) can remove unwanted workstations.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1467663-webstaff-workstation-required

tags: added: pullrequest
Changed in evergreen:
assignee: Bill Erickson (berick) → Jennifer Pringle (jpringle-u)
Bill Erickson (berick) wrote :

Pullrequest added. Jennifer, I assigned the bug back to you. Hope that's OK :)

Jennifer Pringle (jpringle-u) wrote :

I tested this and the workstation admin page is great. If you login without a workstation you are directed to the admin page to create one. You can easily switch between workstations and remove ones that are no longer needed.

I have a question and an issue I encountered while testing.

Question
1. Should the wording on the workstation admin page be "Register a New Workstation For This Browser"/"Workstations Registered With This Browser" rather than "Register a New Workstation For This Computer"/"Workstations Registered With This Computer" since you can have different sets of workstations registered if you are using multiple browsers (not sure if there is a use case for using the client in two different browsers on the same computer)

Issue
1. Org unit scoping isn't being applied. You can currently register a workstation at any org unit regardless of what branch is set as your working location. You can also currently login using a workstation registered to an org unit that doesn't match your working location.
Expected Behaviour: A user should only be able to register a workstation to an org unit that matches their working location(s). A user should only be able to log in to a workstation that is registered to an org unit that matches their working location(s).

Bill Erickson (berick) wrote :

Thanks for testing, Jennifer! Some responses...

Question 1: I like your suggested wording. I'll look at that.

Issue 1: Users can register a workstation at any location within the scope of their REGISTER_WORKSTATION permission. This starts at the working location, but may extend to other branches based on the depth the permission is applied.

The code should be preventing access to org units not covered by permissions by disabling them in the selector. Can you confirm the user you are testing with is able to register at org units that are not within permission range?

Similarly, a staff user can log in to any location within the scope of their STAFF_LOGIN permission. It's not necessarily limited to only their working location. Are you able to log in at a location that's not covered by the permission scope?

Jennifer Pringle (jpringle-u) wrote :

Turns out REGISTER_WORKSTATION is set to Consortium in the Concerto data set. I changed it to Branch on the test server and the user is now only able to register a workstation to the branch(es) that match their working location(s).

STAFF_LOGIN is also set to Consortium which explains users logging into other branches. When I switched STAFF_LOGIN to System or Branch login stopped working for everyone except global admins. On our 2.10 training server we have STAFF_LOGIN set to Consortium. I tested changing it to Library and login stopped working there as well. So I think this is normal.

One other peculiarity I ran into, that may be by design, is that Global Admins can delete all workstations and then use the client without a registered workstation. Other users cannot delete (Remove) the workstation they are currently logged in with.

Bill Erickson (berick) wrote :

Thanks for confirming, Jennifer.

The users that cannot remove a workstation should be the same ones that cannot register a workstation, at least not at the location in question. There should be nothing special about Global Admins, except that they typically have broader permissions.

I suppose when the last workstation is removed, we should prevent the user from navigating to any pages besides the workstation admin page. It doesn't do any harm, since they are still authenticated with their original workstation, but I could see that causing confusion. I'll look at that, too.

Jennifer Pringle (jpringle-u) wrote :

Just to clarify, in my testing the users who could not remove the workstation they were currently logged in with could remove any other workstations they had registered. It was just the global admins who could remove all workstations.

I think it's a good idea to prevent the user from being able to navigate to any other page if the last workstation has been removed, just to prevent confusion. However, assuming the ability to delete all workstations is something that only global admins have (as my testing indicates) I don't think it's a major issue.

Bill Erickson (berick) wrote :

Thanks for the feedback, Jennifer. I suspect the Global Admin issue still has something to do with permissions. (It's the only group in concerto that has the EVERYTHING permission, for example). That said, I was able to remove all workstations using a Catalog Admin account (br1mclark) on my concerto-based test system. It's unclear to me why you can't do the same. The JS console may have more to offer there as far as debugging goes.

I have force-pushed some updates and squashed commits back to the same branch. I have incorporated the suggested wording changes. I've also refactored the code that redirects to the workstation admin page to simplify the logic. Basically, a user must be authenticated with a workstation that has a matching registration in the browser. Otherwise, they are directed to the workstation admin page to remedy the situation.

Mike Rylander (mrylander) wrote :

Just a note: I've picked this into the sprint4 collab branch for further testing and future inclusion.

Bill Erickson (berick) wrote :

Thanks, Mike! Let me know if you encounter any issues.

Changed in evergreen:
assignee: Jennifer Pringle (jpringle-u) → nobody
Kathy Lussier (klussier) wrote :

I've tested this on webby and concur with Jennifer that the new workstation admin page is great. I also tested the original bug, where I tried to log in with a workstation that no longer exists in the database. The system handled this scenario well. I also tested Bill's additions to the code where the user is redirected to the workstation registration page whenever they try to use the client without a registered workstation. It works for me!

Mike Rylander (mrylander) wrote :

I'm going to mark this as fix-committed, since it's in the branch that will definitely be merged to master ... it's in, IOW.

Changed in evergreen:
status: Confirmed → Fix Committed
Changed in evergreen:
milestone: 2.next → 2.12-beta
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers