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

Issue 43278- display forbidden exception message in error handling page #2472

Merged

Conversation

labkey-ankurj
Copy link
Contributor

Rationale

Display forbidden exception message in error page instead of the general permission exception message.

Related Pull Requests

@labkey-ankurj
Copy link
Contributor Author

@labkey-adam I couldn't find the server side callers to LoadLibraryAction and LogClientExceptionAction. The ones I found were client side and the caller to those actions doesn't pass the container.

Copy link
Contributor

@labkey-adam labkey-adam left a comment

Choose a reason for hiding this comment

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

A key change mentioned in the issue is to remove the @IgnoresForbiddenProjectCheck annotation on both actions. And test. I had to add the annotation was because these actions were getting called in a specific folder/project instead of root. Since you've made no changes to the callers I suspect they're still NOT root-based URLs.

@labkey-adam
Copy link
Contributor

Actually, hold on... let me pull this down locally. Something is strange with the isForbiddenProject handling.

@labkey-adam
Copy link
Contributor

Sorry, @labkey-ankurj , I was mistaken. Turns out the "forbidden project" check was NOT failing originally because the invocation was to a different project. It forbid an admin impersonating in a project from accessing root, which is arguably too strict, but that's what's it's doing. Let's forget about removing the annotation... your code is good!

But I'd like to make my follow-on change to this branch... I'll let you know when my change is ready.

@labkey-adam labkey-adam self-requested a review July 19, 2021 23:56
Copy link
Contributor

@labkey-adam labkey-adam left a comment

Choose a reason for hiding this comment

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

Looks good. Let me make my follow-on commit here.

@labkey-ankurj labkey-ankurj merged commit 103561f into release21.7-SNAPSHOT Jul 20, 2021
@labkey-ankurj labkey-ankurj deleted the 21.7_fb_Issue43278_forbiddenMessage branch July 20, 2021 17:30
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.

None yet

2 participants