Skip to content

Fix an issue with the navigation_allowed permission. - #3132

Merged
somiaj merged 1 commit into
openwebwork:developfrom
drgrice1:fix-navigation-allowed-permission-issue
Aug 20, 2026
Merged

Fix an issue with the navigation_allowed permission.#3132
somiaj merged 1 commit into
openwebwork:developfrom
drgrice1:fix-navigation-allowed-permission-issue

Conversation

@drgrice1

Copy link
Copy Markdown
Member

Do not let a user view another set other than the one in the session if a user does not have the navigation_allowed permission. Generally, the only way this will happen is if the user modifies the URL in the browser to try to change to a different set. Currently that works, and it shouldn't.

@drgrice1
drgrice1 force-pushed the fix-navigation-allowed-permission-issue branch from 2aa915a to 5c20ed2 Compare August 13, 2026 19:56
@drgrice1
drgrice1 force-pushed the fix-navigation-allowed-permission-issue branch 2 times, most recently from 64d980f to e675cf1 Compare August 19, 2026 18:07
@somiaj

somiaj commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

I don't have LTI fully setup on my develop server, but I noticed the following as an instructor:

In course configuration set navigation_allowed to nobody.

Go to set manager, and click on the problem number for a set to get into the set details editor, from there the link to the set appears under the greyed out assignments page. Click on that to get to the link to the set (or typing in the url directly) gives a warning:

Use of uninitialized value in string ne at /home/jaimos/git/openwebwork/webwork2/lib/WeBWorK/Authz.pm line 437.

Now this is not something anyone should be doing, but the error states that there could be a chance that either $c->authen->session->{set_id} could be undefined. Unsure if there is a way that could happen.

Comment thread lib/WeBWorK/Authz.pm Outdated
return $c->maketext("Requested set '[_1]' is not available yet.", $setName);
}

if (!$self->hasPermissions($userName, 'navigation_allowed') && $c->authen->session->{set_id} ne $setName) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changing this to:

if (!$self->hasPermissions($userName, 'navigation_allowed') && ($c->authen->session->{set_id} // '') ne $setName) {

Fixes the warning I ran into.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I made that change, but I am certain that if you start changing various permissions to nobody that really shouldn't be, and that don't make sense to be nobody, you will find lots of other more serious problems with the code not being prepared to deal with that.

Do not let a user view another set other than the one in the session if
a user does not have the `navigation_allowed` permission.  Generally,
the only way this will happen is if the user modifies the URL in the
browser to try to change to a different set. Currently that works, and
it shouldn't.
@drgrice1
drgrice1 force-pushed the fix-navigation-allowed-permission-issue branch from e675cf1 to 1f8ad10 Compare August 20, 2026 17:46
@somiaj

somiaj commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Agreed, this permission doesn't work well for anything above login_proctor which the help states to use if you want to restrict student navigation.

I was just worried their maybe other cases where $c->authen->session->{set_id} could be undefined for a student user.

@somiaj
somiaj merged commit 40133e4 into openwebwork:develop Aug 20, 2026
2 checks passed
@drgrice1
drgrice1 deleted the fix-navigation-allowed-permission-issue branch August 20, 2026 19:05
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