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

Can't apply [Authorize] on page handler methods #18676

Merged
merged 19 commits into from
Jun 9, 2020

Conversation

Rick-Anderson
Copy link
Contributor

@Rick-Anderson Rick-Anderson commented Jun 5, 2020

Fixes #18673

Internal review URL

@pranavkm Should I move your sample repo into the this repo?

@Rick-Anderson Rick-Anderson requested a review from pranavkm June 5, 2020 00:50
@Rick-Anderson
Copy link
Contributor Author

@pranavkm quick review when you have time will save you time in the long run :)

@pranavkm
Copy link
Contributor

pranavkm commented Jun 6, 2020

Should I move your sample repo into the this repo?

Sure, that would be great.

aspnetcore/security/authorization/simple.md Outdated Show resolved Hide resolved
@Rick-Anderson
Copy link
Contributor Author

Could we call out that this does not work in conjunction

Sorry @pranavkm I mean to add that. I got really ambitious on my last commit so you'll need another look and maybe @mkArtakMSFT too. I'm hoping what I've written will cut down on all that traffic on dotnet/aspnetcore#8737

@Rick-Anderson
Copy link
Contributor Author

@pranavkm I cloned https://github.com/pranavkm/PageHandlerAuth and copied the code to this repo. But on inspection, in dotnet/aspnetcore#8737 you show the code for your first commit. Just need to verify I need to copy over the code from your first commit. You 2nd commit is much different.



[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
public class AuthorizePageHandlerAttribute : Attribute, IAuthorizeData
Copy link
Contributor

Choose a reason for hiding this comment

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

I edited this to be more inline with what was shared in the issue (based on the initial commit). Could you move this to a new file? If you're able to try it out, that would be great. I edited this on GitHub, so I'm not sure if all this compiles \ works correctly.

@Rick-Anderson
Copy link
Contributor Author

Rick-Anderson commented Jun 8, 2020

@blowdart please review Internal review URL and this PR.

@mkArtakMSFT I added the following on my own, please approve:

  • When possible, moving shared content to a partial view is the preferred approach.
  • There are no plans to support the AuthorizeAttribute on Razor Page handlers.

@pranavkm
Copy link
Contributor

pranavkm commented Jun 8, 2020

@mkArtakMSFT I added the following on my own, please approve:

When possible, moving shared content to a partial view is the preferred approach.
There are no plans to support the AuthorizeAttribute on Razor Page handlers.

Yeah, that seems fine to say.

Co-authored-by: Pranav K <prkrishn@hotmail.com>
@Rick-Anderson Rick-Anderson merged commit dd2f0ca into master Jun 9, 2020
@Rick-Anderson Rick-Anderson deleted the rp/auth/page/handlers/ra branch June 9, 2020 23:44
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.

Doc no [Authorize] on RP methods, provide sample
2 participants