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

Evolve API authentication to omit it on endpoints intended to be open #9466

Closed
GPortas opened this issue Mar 22, 2023 · 3 comments · Fixed by #10131
Closed

Evolve API authentication to omit it on endpoints intended to be open #9466

GPortas opened this issue Mar 22, 2023 · 3 comments · Fixed by #10131
Assignees
Labels
Feature: API NIH OTA: 1.7.1 (reArchitecture) 7 | 1.7.1 | Research & architecture for separating backend and frontend to enable a flexible, sca... Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) User Role: API User Makes use of APIs
Milestone

Comments

@GPortas
Copy link
Contributor

GPortas commented Mar 22, 2023

Overview of the Feature Request

After #9293 refactor, it has become more visible that there are endpoints that, despite being intended to be open, triggers user authentication, when it is not required.

This behavior already existed before the Auth Filter refactor, but it is now more visible by having the @AuthRequired annotation. Before the refactor, credential filtering was executed via the AbstractApiBean.response(DataverseRequestHandler hdl) method, which was called from several endpoints, a method which in turn called the findUserOrDie method.

These methods no longer exist, since the logic is now moved to the Auth Filter, and the same endpoints which used those methods now they are wrapped by the Auth Filter.

The goal of this issue is to simplify authentication by omitting the auth filter on endpoints that do not require user authentication. This makes the API code more understandable for developers and improves performance by bypassing the auth filter when it's not needed.

Example endpoint: /api/info/version.

What kind of user is the feature intended for?
API User, developers

What inspired the request?
Slack discussion about confusion when seeing endpoint /api/info/version marked with AuthRequired

What existing behavior do you want changed?
API authentication

Any brand new behavior do you want to add to Dataverse?
No

Any open or closed issues related to this feature request?

@GPortas GPortas added Feature: API User Role: API User Makes use of APIs NIH OTA: 1.7.1 (reArchitecture) 7 | 1.7.1 | Research & architecture for separating backend and frontend to enable a flexible, sca... labels Mar 22, 2023
@pdurbin
Copy link
Member

pdurbin commented Mar 22, 2023

If we implement this we should probably follow up on this thread where we explained that @AuthRequired is required: https://groups.google.com/g/dataverse-dev/c/JC_ZakBG_d8/m/uQHADVtBAQAJ

@GPortas GPortas added the Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) label Apr 11, 2023
@cmbz cmbz added this to the 6.1 milestone Sep 25, 2023
@cmbz
Copy link

cmbz commented Sep 25, 2023

2023/09/25: Added to 6.1 milestone as per conversation during prioritization meeting.

qqmyers added a commit to GlobalDataverseCommunityConsortium/dataverse that referenced this issue Sep 25, 2023
@cmbz cmbz added this to ▶ SPRINT READY in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) via automation Oct 16, 2023
@scolapasta scolapasta moved this from ▶ SPRINT READY to This Sprint 🏃‍♀️ 🏃 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Nov 15, 2023
@scolapasta scolapasta moved this from This Sprint 🏃‍♀️ 🏃 to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Nov 17, 2023
@pdurbin
Copy link
Member

pdurbin commented Nov 17, 2023

The conversation in Slack that lead to this issue: https://iqss.slack.com/archives/C010LA04BCG/p1678391881465749

@jp-tosca jp-tosca moved this from ▶ SPRINT READY to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Nov 27, 2023
pdurbin added a commit to jp-tosca/dataverse that referenced this issue Nov 28, 2023
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from IQSS Team - In Progress 💻 to Done 🚀 Nov 28, 2023
@scolapasta scolapasta moved this from Done 🚀 to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: API NIH OTA: 1.7.1 (reArchitecture) 7 | 1.7.1 | Research & architecture for separating backend and frontend to enable a flexible, sca... Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) User Role: API User Makes use of APIs
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

4 participants