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.

Sign up for GitHub

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

Jump to bottom

Allow access to health endpoint as per defined roles #2632

Merged
merged 66 commits into from
Apr 4, 2025
Merged

Allow access to health endpoint as per defined roles #2632

merged 66 commits into from
Apr 4, 2025

Conversation

sezal98
Copy link
Contributor

@sezal98 sezal98 commented Mar 24, 2025 •
edited
Loading

Why make this change?

Closes #2531

What is this change?

This PR involves creating another parameter in the runtime config which defines the allowed roles for the health endpoint.

  • It created src/Config/Converters/RuntimeHealthOptionsConvertorFactory.cs which takes care of serialization and deserialization of runtime config custom way.
  • The incoming role and token are taken up to firct check if the role in the header is matching with the allowed roles in the runtime config.
  • Further this role and token are passed to execute the REST and GraphQL queries for DAB.

Scenarios

  1. Allowed Roles are not configured.
  • Mode is Development - we allow all requests
  • Mode is Production - we dont allow any requests. We show 403
  1. Allowed Roles are configured
  • Mode is Development or Production - we performt the role check in allowed roles.

How was this tested?

  • Integration Tests
  • Unit Tests

Sample Request(s)

Roles: ["authenticated"]
Mode: Development or Production
image
image

Roles: Not Configured
Mode: Development
image

Mode: Production
image

Roles: ["admin"]
Mode: Development or Production
image

sezalchug and others added 30 commits February 26, 2025 20:55
Modify Config Files according to health check endpoint
ee1561d
formatting
7a1734d
@sezal98
modify the if condition for comprehensive check
49daea3
Initial commit
95e4777
resolving comments
374ce72
Merge branch 'dev/sezalchug/healthConfigFiles' of https://github.com/…
e2b23c4 
…Azure/data-api-builder into dev/sezalchug/healthConfigFiles
formatting
ab2258b
Merge branch 'dev/sezalchug/healthConfigFiles' into dev/sezalchug/hea…
6125132 
…lthCheckExecution
merging into config changes
c7f0823
UTs
9b6de0b
nit changes and UTs and default value
e95d6ae
formatting
e8cd0b5
response file sample
74042e1
based on discussions in the meeting
e1d51aa
nits
8207dc5
snapshots
b7e7218
Merge branch 'main' of https://github.com/Azure/data-api-builder into…
521221f 
… dev/sezalchug/healthCheckExecution
remove value
b574648
format
cb95c11
build
9ae3150
tests
b7fbb77
nit comments
daa4742
revert add entity snapshots
80ab25c
module initializer
c5eb328
update entity tests revert
bc2adb8
end to end tests
5fc5cc6
serialization and deserialization changes
433010d
nits
4cb4fb0
Role changes in Health endpoint
2c20d1a
formatting
ed07ac0
Aniruddh25
Aniruddh25 requested changes Apr 2, 2025
View reviewed changes
Copy link
Contributor

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Need more tests/assertion on the Rest Query/GraphQL query execution.

@Aniruddh25
Copy link
Contributor

nit: please provide a better title to the PR that explains what this PR is doing.

@sezal98 sezal98 requested a review from Aniruddh25 April 3, 2025 20:11
RubenCerna2079
RubenCerna2079 previously approved these changes Apr 4, 2025
View reviewed changes
Copy link
Contributor

@RubenCerna2079 RubenCerna2079 left a comment

Choose a reason for hiding this comment

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

LGTM

@Aniruddh25 Aniruddh25 changed the title Roles Health Check Allow access to health endpoint as per defined roles Apr 4, 2025
@Aniruddh25
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@RubenCerna2079
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

Aniruddh25
Aniruddh25 approved these changes Apr 4, 2025
View reviewed changes
Copy link
Contributor

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for addressing all the concerns specially about the role headers!

@RubenCerna2079
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@RubenCerna2079 RubenCerna2079 merged commit ebbe989 into main Apr 4, 2025
11 checks passed
@RubenCerna2079 RubenCerna2079 deleted the dev/sezalchug/rolesHealthCheck branch April 4, 2025 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aniruddh25 Aniruddh25 Aniruddh25 approved these changes

@RubenCerna2079 RubenCerna2079 RubenCerna2079 approved these changes

@aaronburtle aaronburtle Awaiting requested review from aaronburtle aaronburtle is a code owner

@ravishetye ravishetye Awaiting requested review from ravishetye ravishetye is a code owner

@rohkhann rohkhann Awaiting requested review from rohkhann rohkhann is a code owner

@neeraj-sharma2592 neeraj-sharma2592 Awaiting requested review from neeraj-sharma2592 neeraj-sharma2592 is a code owner

@sourabh1007 sourabh1007 Awaiting requested review from sourabh1007 sourabh1007 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[HealthEndpoint] Roles Support
3 participants
@sezal98 @Aniruddh25 @RubenCerna2079