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

feat: Create new api role in Hasura #2294

Merged
merged 3 commits into from
Oct 12, 2023
Merged

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Oct 11, 2023

What does this PR do?

  • Adds a new api role in Hasura
  • Adds introspection test coverage for user role

What's the motivation behind this?

I'm working towards https://trello.com/c/qSCoDRAO/2650-implement-editor-permissions-throughout-the-api which has two main aims -

  • When a request to the API is initiated by a user, use their permission level throughout the call stack
  • When a request to the API is initiated by an internal service (e.g. Hasura), use appropriate permissions (not admin token with full "God mode")

The first step has been pretty well started in previous PRs. Adding an api role will allow us to scope service → API requests appropriately when a service has authenticated to the API (e.g. through the useHasuraAuth() middleware).

The permission set outlined here is based on queries currently made by the two admin GraphQL clients currently used in the API.

Next steps...

  • Drop $admin client from API
  • Drop adminGraphQLClient from API
  • Replace them with a role-scoped client. Either a user's role, or the new api role for service → API requests.
  • Where a user-initiated request requires a "side effect" (such as adding an audit log), I think it would be appropriate to call the API-scoped client. An alternative could be using backend only mutations but this feels like a more complex solution.

I'll likely split the above into a few PRs just to make them a bit more manageable.

Nice to have?

Porting the introspection tests from JavaScript / Jest to Gherkin could be a really nice tidy up. I think it would be approx 10% of the number of lines of code that we currently have and be a lot easier to maintain and parse. This is not a very high priority tbh!

@github-actions
Copy link

github-actions bot commented Oct 11, 2023

🤖 Hasura Change Summary compared a subset of table metadata including permissions:

Updated Tables (14)

  • public.bops_applications permissions:

    insert select update delete
    api
    30 added column permissions
    insert select update
    api ➕ bops_id
    ➕ created_at
    ➕ destination_url
    ➕ id
    ➕ req_headers
    ➕ request
    ➕ response
    ➕ response_headers
    ➕ sanitised_at
    ➕ session_id
    ➕ bops_id
    ➕ created_at
    ➕ destination_url
    ➕ id
    ➕ req_headers
    ➕ request
    ➕ response
    ➕ response_headers
    ➕ sanitised_at
    ➕ session_id
    ➕ bops_id
    ➕ created_at
    ➕ destination_url
    ➕ id
    ➕ req_headers
    ➕ request
    ➕ response
    ➕ response_headers
    ➕ sanitised_at
    ➕ session_id
  • public.email_applications permissions:

    insert select update delete
    api
    24 added column permissions
    insert select update
    api ➕ created_at
    ➕ id
    ➕ recipient
    ➕ request
    ➕ response
    ➕ sanitised_at
    ➕ session_id
    ➕ team_slug
    ➕ created_at
    ➕ id
    ➕ recipient
    ➕ request
    ➕ response
    ➕ sanitised_at
    ➕ session_id
    ➕ team_slug
    ➕ created_at
    ➕ id
    ➕ recipient
    ➕ request
    ➕ response
    ➕ sanitised_at
    ➕ session_id
    ➕ team_slug
  • public.flow_document_templates permissions:

    insert select update delete
    api
    2 added column permissions
    insert select update
    api ➕ document_template
    ➕ flow_id
  • public.flows permissions:

    insert select update delete
    api
    31 added column permissions
    insert select update
    api ➕ copied_from
    ➕ created_at
    ➕ creator_id
    ➕ data
    ➕ id
    ➕ settings
    ➕ slug
    ➕ team_id
    ➕ updated_at
    ➕ version
    ➕ copied_from
    ➕ created_at
    ➕ creator_id
    ➕ data
    ➕ data_merged
    ➕ id
    ➕ settings
    ➕ slug
    ➕ team_id
    ➕ updated_at
    ➕ version
    ➕ copied_from
    ➕ created_at
    ➕ creator_id
    ➕ data
    ➕ id
    ➕ settings
    ➕ slug
    ➕ team_id
    ➕ updated_at
    ➕ version
  • public.lowcal_sessions permissions:

    insert select update delete
    api
    22 added column permissions
    insert select update
    api ➕ created_at
    ➕ data
    ➕ deleted_at
    ➕ email
    ➕ flow_id
    ➕ has_user_saved
    ➕ id
    ➕ locked_at
    ➕ sanitised_at
    ➕ submitted_at
    ➕ updated_at
    ➕ created_at
    ➕ data
    ➕ deleted_at
    ➕ email
    ➕ flow_id
    ➕ has_user_saved
    ➕ id
    ➕ locked_at
    ➕ sanitised_at
    ➕ submitted_at
    ➕ updated_at
  • public.payment_requests permissions:

    insert select update delete
    api
    30 added column permissions
    insert select update
    api ➕ applicant_name
    ➕ created_at
    ➕ govpay_payment_id
    ➕ id
    ➕ paid_at
    ➕ payee_email
    ➕ payee_name
    ➕ payment_amount
    ➕ session_id
    ➕ session_preview_data
    ➕ applicant_name
    ➕ created_at
    ➕ govpay_payment_id
    ➕ id
    ➕ paid_at
    ➕ payee_email
    ➕ payee_name
    ➕ payment_amount
    ➕ session_id
    ➕ session_preview_data
    ➕ applicant_name
    ➕ created_at
    ➕ govpay_payment_id
    ➕ id
    ➕ paid_at
    ➕ payee_email
    ➕ payee_name
    ➕ payment_amount
    ➕ session_id
    ➕ session_preview_data
  • public.payment_status permissions:

    insert select update delete
    api
    7 added column permissions
    insert select update
    api ➕ amount
    ➕ created_at
    ➕ flow_id
    ➕ payment_id
    ➕ session_id
    ➕ status
    ➕ team_slug
  • public.planning_constraints_requests permissions:

    insert select update delete
    api
    5 added column permissions
    insert select update
    api ➕ created_at
    ➕ destination_url
    ➕ id
    ➕ response
    ➕ session_id
  • public.published_flows permissions:

    insert select update delete
    api
    12 added column permissions
    insert select update
    api ➕ created_at
    ➕ data
    ➕ flow_id
    ➕ id
    ➕ publisher_id
    ➕ summary
    ➕ created_at
    ➕ data
    ➕ flow_id
    ➕ id
    ➕ publisher_id
    ➕ summary
  • public.reconciliation_requests permissions:

    insert select update delete
    api
    5 added column permissions
    insert select update
    api ➕ created_at
    ➕ id
    ➕ message
    ➕ response
    ➕ session_id
  • public.team_members permissions:

    insert select update delete
    api
    4 added column permissions
    insert select update
    api ➕ id
    ➕ role
    ➕ team_id
    ➕ user_id
  • public.teams permissions:

    insert select update delete
    api
    12 added column permissions
    insert select update
    api ➕ boundary
    ➕ boundary_bbox
    ➕ created_at
    ➕ domain
    ➕ id
    ➕ name
    ➕ notify_personalisation
    ➕ settings
    ➕ slug
    ➕ submission_email
    ➕ theme
    ➕ updated_at
  • public.uniform_applications permissions:

    insert select update delete
    api
    27 added column permissions
    insert select update
    api ➕ created_at
    ➕ destination
    ➕ id
    ➕ idox_submission_id
    ➕ payload
    ➕ response
    ➕ sanitised_at
    ➕ submission_reference
    ➕ xml
    ➕ created_at
    ➕ destination
    ➕ id
    ➕ idox_submission_id
    ➕ payload
    ➕ response
    ➕ sanitised_at
    ➕ submission_reference
    ➕ xml
    ➕ created_at
    ➕ destination
    ➕ id
    ➕ idox_submission_id
    ➕ payload
    ➕ response
    ➕ sanitised_at
    ➕ submission_reference
    ➕ xml
  • public.users permissions:

    insert select update delete
    api
    7 added column permissions
    insert select update
    api ➕ created_at
    ➕ email
    ➕ first_name
    ➕ id
    ➕ is_platform_admin
    ➕ last_name
    ➕ updated_at

@github-actions
Copy link

github-actions bot commented Oct 11, 2023

Removed vultr server and associated DNS entries

@DafyddLlyr DafyddLlyr requested a review from a team October 11, 2023 09:55
@DafyddLlyr DafyddLlyr marked this pull request as ready for review October 11, 2023 09:55
Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All really clear, thanks for continuing to pick this up !

@DafyddLlyr DafyddLlyr merged commit b15ccbd into main Oct 12, 2023
10 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/admin-api-permissions branch October 12, 2023 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants