Feature-1816: Metacat Admin MN Privileges (auth.administrators) - Resolve Broken Tests #1824
Conversation
…' method when session is null and adminUser is null or empty, and add new junit tests to 'D1AuthhelperTest'
… privileges and added additional comments & javadocs to improve clarity
…ion is not used, and improve javadoc wording
…th debugging and clarity
…or a metacat admin to remove anti-pattern
|
Thank you for reviewing this PR and meeting with me @taojing2002! I believe I have addressed the issues raised here. Additionally, I have refactored |
|
All tests passed! Great! Overall it looks good. But I added some comments. |
| throw new NotAuthorized("0000", "Session is null."); | ||
| } | ||
| // If there is no subject or if subject is empty, session is not authorized. | ||
| String sessionSubject = session.getSubject().getValue(); |
There was a problem hiding this comment.
If session.getSubject is null, the method will get null exception.
There was a problem hiding this comment.
I have wrapped the relevant block in a try statement to catch this NullPointerException and throw a new NotAuthorized exception with the message "Session subject is not set."
…AdminAuthorization' and add new junit test
…min in the auth.administrators's list, and new junit test for 'doAuthUpdate' with mismatched auth mnode
|
Hi @taojing2002, I have addressed your feedback. Can you please let me know if I misunderstood the second point:
I am assuming that you mean a check to ensure that if the |
… 'NotAuthorized' exception to the last if statement when checking auth for metacat admins
…id session and respective subject value into new method 'checkSessionAndGetSubjectValue'
Hi @taojing2002, I have amended
doAdminAuthorizationinD1AuthHelperto throwNotAuthorizedexceptions when session is null, and when session is not null but subject is empty or null. Can you please let me know if you are experiencing any other errors?