improved documentation #1776
improved documentation #1776
Conversation
| /// that will be persisted through the authentication process. The ProtocolMessage can also be used to add or customize | ||
| /// Invoked before redirecting to the identity provider to authenticate. This can be used to set | ||
| /// <see cref="RedirectContext.ProtocolMessage"/> | ||
| /// that will be persisted through the authentication process. The ProtocolMessage can also be used to add or alter |
There was a problem hiding this comment.
What were you trying to convey? Or did you just want to add the cref?
This is confusing two ideas. This sentence was specifically about the ability to round trip user State. Customizing the RedirectContext.ProtocolMessage is already covered in the next sentence.
|
Ok I'll have another look tonight and fix. |
|
@Tratcher ok addressed and added some more docs. |
There was a problem hiding this comment.
I think I see what you're going for here but I don't think it makes sense to add these kinds of comments in the API docs. This seems more appropriate for a concept doc.
We don't have a generic OIDC doc, maybe we need one. The closest we have are the AAD docs: https://docs.microsoft.com/en-us/aspnet/core/security/authentication/azure-active-directory/?view=aspnetcore-2.1
| /// Gets or Sets the URI used for the redirect operation. | ||
| /// </summary> | ||
| /// <remarks>If you cannot let ASP.NET rewrite the <see cref="HttpRequest.PathBase"/> | ||
| /// for you, such as through <c>ForwardedHeadersOptions</c></remarks> you can use this. |
There was a problem hiding this comment.
This often isn't a local uri, so these comments don't make sense.
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets or Sets the URI used for the redirect operation. |
There was a problem hiding this comment.
This isn't the right class that goes with OIDC events. You're looking for https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/Events/RedirectContext.cs.
| /// </summary> | ||
| /// <remarks> | ||
| /// While this can be used to rewrite the redirect URLs if the app is behind a proxy server, its best to use | ||
| /// <c>ForwardedHeadersOptions</c>. Please see: https://docs.microsoft.com/en-us/aspnet/core/host-and-deploy/proxy-load-balancer?view=aspnetcore-2.1. |
There was a problem hiding this comment.
We can't use direct links in doc comments, the links are unstable. If it were critical we'd use a forward link.
|
@Tratcher can you create an aka.ms link for the doc, fix the bad XML, and merge? |
|
Closing due to lack of follwup. The comments about forwarders do not belong here and the other changes are aesthetic only. |
No description provided.