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

Add an access_test method to API to dev #402

Merged
merged 5 commits into from
Dec 6, 2022
Merged

Conversation

gregcorbett
Copy link
Member

Subject to #401 being reviewed, this PR contains no unreviewed code.

Allow single quote in user identifiers to master
- so Service Owners can verify their hosts will retain access to
  the API after access to personal data is restricted.
- Requires a change to authByIdentifier and isRestrictPDByRole
  to force the restrictions for the test method regardless of the
  wider API access rules
- I haven't included Users in this API check as I don't want to
  encourage human users talking to the API.
@gregcorbett gregcorbett requested a review from a team as a code owner November 29, 2022 10:21
@gregcorbett gregcorbett self-assigned this Nov 29, 2022
@ghost
Copy link

ghost commented Nov 29, 2022

If xml is returned, shouldn't it be roughly complete? Something like

<?xml version="1.0" encoding="UTF-8"?>
<results identity="/C=UK/O=eScience/OU=CLRC/L=RAL/CN=my name">
<authorized>true</authorized>
</results>

maybe, consistent with the other method returns? Lazy parsing of the result could still just grep for <authorized>true (or success, whatever) but there is a chance of returning at least the DN/id or other 'useful' diagnostics?

@gregcorbett
Copy link
Member Author

Sounds good, please do suggest the code you put together that generated that XML

Ian Neilson and others added 3 commits November 30, 2022 14:51
- as I have touched it in this PR and a PR in dev updates the
  code style of the wider file.
@gregcorbett gregcorbett merged commit 5e52edf into dev Dec 6, 2022
@gregcorbett gregcorbett deleted the hotfix-5.10-access_test branch December 6, 2022 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant