Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Handle unregistered users in BearerTokenAuthMechanism and implement user registration mechanism #10972
base: develop
Are you sure you want to change the base?
Handle unregistered users in BearerTokenAuthMechanism and implement user registration mechanism #10972
Changes from 50 commits
5b9e67e
d61d5bf
e42eb5b
89b3119
300e041
ba70a04
80ad5a4
2ca0722
e382a15
fd68cd2
6a7a3e1
424dcbf
3539677
43805f0
63790db
37afa98
0041552
a021c9b
bf601e6
6083982
e544221
753f6eb
86355e3
15b78bb
415e23b
b1901c2
fadebca
b993ba1
32f5fec
c0c6704
a064a7b
672582a
4bc58f6
4536f91
99ce940
386b6ac
b5d40ad
dce7edf
9a62528
396aaa4
0f2cfdc
52a5a9e
047a14c
cc86a83
c3de7d7
25cdf98
5d39ac1
a438c8a
9ba377e
b00ac7f
c269d3c
73c4079
f99732b
dbfe40d
7495364
4ae119c
335e40a
4ca6070
07794f3
7d88c8e
ae58595
f360b91
16f8e04
cc99a8b
353da1d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What terms? Is it https://guides.dataverse.org/en/latest/api/native-api.html#get-api-terms-of-use-url - if so can that be referenced here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, I am referring to the terms of use of the installation, the ones present in the sign-up form and are configurable in the installation.
I have rephrased the text to: "It is essential to send a JSON that includes the property
termsAccepted
set to true, which indicates that you accept the Terms of Use of the installation. Otherwise, you will not be able to create an account."I don't think I can provide any links to the terms as they are embedded and not present by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, would it make sense to add a property per OIDC provider that automatically sets this value to true? I'm thinking of internal institutional installations where its acceptance is implicit or already covered during account creation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I think some config option like that for terms, affiliation, and role would make sense, but I'd definitely suggest a new PR rather holding this up and making it even bigger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understood, in the comments, you suggested that ignoring the values could be confusing. Would it be better to just fail if you send info that is already in the token? (+1 from me)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I just updated it.