Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Move auth into middleware #3133

Merged
merged 21 commits into from
Jun 7, 2023
Merged

Move auth into middleware #3133

merged 21 commits into from
Jun 7, 2023

Conversation

Porges
Copy link
Member

@Porges Porges commented May 25, 2023

Closes #2098.

This cleans up the authentication a bit; after this change we have two stages in the middleware pipeline:

  • AuthenticationMiddleware reads the JWT token (it does not validate it, this is done by the Azure Functions service) and stores it in FunctionContext.Items["ONEFUZZ_USER_INFO"]
  • AuthorizationMiddleware checks the user info against the [Authorize] attribute to see if the user has the required permissions
  • Functions can read the user info from the FunctionContext if needed

The authorize attribute can be [Authorize(Allow.User)] or Allow.Agent or Allow.Admin. The Admin case is new and allows this to be declaratively specified rather than being checked in code. We have several functions which could be changed to use this (e.g. Pool POST/DELETE/PATCH, Scaleset POST/DELETE/PATCH), but I have only changed one so far (JinjaToScriban).

One of the benefits here is that this simplifies the test code a lot: we can set the desired user info directly onto our (Test)FunctionContext rather than having to supply a fake that pretends to parse the token from the HTTP request. This will also have benefits when running the service locally for testing purposes (refer to internal issue).

The other benefit is the ability to programmatically read the required authentication for each function, which may help with Swagger generation.

@codecov-commenter
Copy link

codecov-commenter commented May 25, 2023

Codecov Report

Merging #3133 (ac8cb9b) into main (b44cff5) will decrease coverage by 0.34%.
The diff coverage is 23.61%.

@@            Coverage Diff             @@
##             main    #3133      +/-   ##
==========================================
- Coverage   29.97%   29.63%   -0.34%     
==========================================
  Files         327      331       +4     
  Lines       38816    38773      -43     
==========================================
- Hits        11634    11492     -142     
- Misses      27182    27281      +99     
Impacted Files Coverage Δ
...ervice/ApiService/Auth/AuthenticationMiddleware.cs 0.00% <0.00%> (ø)
...Service/ApiService/Auth/AuthorizationMiddleware.cs 0.00% <0.00%> (ø)
...piService/ApiService/Functions/AgentCanSchedule.cs 0.00% <0.00%> (ø)
...c/ApiService/ApiService/Functions/AgentCommands.cs 0.00% <0.00%> (ø)
src/ApiService/ApiService/Functions/Config.cs 0.00% <0.00%> (ø)
.../ApiService/ApiService/Functions/InstanceConfig.cs 0.00% <0.00%> (ø)
src/ApiService/ApiService/Functions/Negotiate.cs 0.00% <0.00%> (ø)
...c/ApiService/ApiService/Functions/NodeAddSshKey.cs 0.00% <0.00%> (ø)
...c/ApiService/ApiService/Functions/Notifications.cs 0.00% <0.00%> (ø)
...iService/ApiService/Functions/NotificationsTest.cs 0.00% <0.00%> (ø)
... and 29 more

... and 3 files with indirect coverage changes

@Porges Porges marked this pull request as ready for review May 25, 2023 23:04
@Porges Porges changed the title Auth middleware Move auth into middleware May 25, 2023
@Porges
Copy link
Member Author

Porges commented May 25, 2023

Passes check-pr.

@Porges Porges requested review from stishkin, chkeita and tevoinea May 25, 2023 23:07
@Porges
Copy link
Member Author

Porges commented Jun 7, 2023

Did some manual validation after check-pr. All looks good.

@Porges Porges merged commit e448947 into main Jun 7, 2023
@Porges Porges deleted the auth-middleware branch June 7, 2023 01:57
@AdamL-Microsoft AdamL-Microsoft mentioned this pull request Jun 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move "CallIf" into Azure Function middleware
4 participants