9293 - Apply filter-based auth for all API endpoints (2/2)#9360
9293 - Apply filter-based auth for all API endpoints (2/2)#9360kcondon merged 33 commits intoIQSS:developfrom
Conversation
…ted AbstractApiBean method 'response' removed
…serOrDie calls in API endpoint methods
…nd replaced its calls to filter-based logic
…private method to use findUserOrDie method
… from AbstractApiBean in addition to its associated methods
|
Kickoff resizing
|
|
FWIW - In another conversation, it was just pointed out to me that the API calls in the mydata package all appear to use instead of findUser/findAUthenticatedUserOrDie. They should probably be updated to use the filter as well, either here or as a new issue/PR.These may be the only api calls outside the api package - I didn't find any @path annotations anywhere else. |
The only call to the getUserFromIdentifier method used to authenticate a user is wrapped in a DEBUG condition (which is always false and it might be interesting to remove it, since it never gets called), please take a look at the call I'm referring to: The rest of the calls to getUserFromIdentifier are made once the user has been authenticated through another mechanism. These mechanisms are checked separately, not via the old findUserOrDie or findAuthenticatedUserOrDie methods, as you mention. In effect, that endpoint is a candidate for the Auth Filter to be applied on it. By having already identified this, I think it's a good time to also refactor that endpoint in this same PR. FWIW - I see that session authentication is also enabled for such endpoint (See ), I don't know if there is any special reason. Perhaps the endpoint is called or has been called in the past from JSF, so it requires to authenticate by Session Cookie. In any case, the Auth Filter can be applied while also maintaining session authentication for this API resource, if necessary. |
|
@GPortas - Thanks! Sorry for the bad code reference. The real issue that was found is that signedURLs don't work with these calls. It's because this class is checking for a session or api key (I think at ) by itself so it therefore isn't seeing the newer methods like signedURLs and workflow tokens.MyData pre-dates me, but I think you are probably right about the session being accepted - the MyData page apparently uses different technologies and probably calls that API directly. I would guess that use is current and should be kept, but I haven't checked. It does look like removing that if DEBUG statement that is always false would be good code clean-up. |
Great, thank you for the clarification. That issue will be solved by applying the Auth Filter to the endpoint, which includes the newer methods you mention. I'm already working on these changes, to add them to this PR. |
…lter to allow more authentication mechanisms
rtreacy
left a comment
There was a problem hiding this comment.
status codes, messages, more auth, tests
|
@GPortas This pr has the same signedurl issue as part one -encoding? Would a refresh of this branch from develop solve the problem? |
What this PR does / why we need it:
This is a continuation of the refactoring work done for PR #9303, containing the remaining changes to close issue #9293. It is necessary to review and merge PR #9303 before reviewing this PR.
In particular, this PR replaces the calls to the deprecated findUserOrDie and findAuthenticatedUserOrDie methods with the filter-based design and removes the deprecated methods, no longer used by any API endpoint.
Which issue(s) this PR closes:
Closes #9293
Special notes for your reviewer:
Not yet
Suggestions on how to test this:
The behavior of the API must be the same as before, so in addition to the automated IT tests, curl calls to the API using different types of credentials can be used to test the different auth mechanisms.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
Not sure
Additional documentation:
Not yet / Not sure