Skip to content
This repository has been archived by the owner on Dec 13, 2022. It is now read-only.

Documentation Issue at docs/topics/cors.rst #4685

Closed
abarker11101 opened this issue Jul 24, 2020 · 5 comments
Closed

Documentation Issue at docs/topics/cors.rst #4685

abarker11101 opened this issue Jul 24, 2020 · 5 comments
Assignees

Comments

@abarker11101
Copy link

This is not a bug per se, more of a documentation issue. My apologies if I should have categorized it differently.

I previously submitted this change as a pull request (https://github.com/IdentityServer/IdentityServer4/pull/4404/files). Again my apologies, I don't think that was the best way to communicate what I believe to be a documentation issue.

docs/topics/cors.rst indicates that you define Allowed Origins like this:
var cors = new DefaultCorsPolicyService(_loggerFactory.CreateLogger())
{
AllowedOrigins = { "https://foo", "https://bar" }
};
services.AddSingleton(cors);

However, this was not working for me. What I did eventually get working is:
services.AddSingleton((container) => {
var logger = container.GetRequiredService<ILogger>();
return new DefaultCorsPolicyService(logger) {
AllowedOrigins = { "https://foo", "https://bar" }
};
});

I believe the reason that the former approach did not work is that an instance of _loggerFactory is no longer available in Startup.cs->ConfigureServices(). I believe this is due to a change Microsoft made in the approach to configuring logging in .NET Core 3.1.

@Jay-study-nildana
Copy link
Contributor

I am facing the same issue. the original code no longer works.

But the code suggested by @abarker11101 also does not work. it keeps saying that the ILogger provided by Microsoft is not compatible with the ILogger need by DefaultCorsPolicyService.

@abarker11101
Copy link
Author

abarker11101 commented Jul 28, 2020

I think this code may work better:

services.AddSingleton<ICorsPolicyService>((container) => {
    var logger = container.GetRequiredService<ILogger<DefaultCorsPolicyService>>();
    return new DefaultCorsPolicyService(logger) {
        AllowedOrigins = { "https://foo", "https://bar" }
    };
});

@Jay-study-nildana
Copy link
Contributor

@abarker11101 that worked.

@brockallen brockallen added the docs label Sep 7, 2020
@brockallen brockallen added this to the 4.0.5 milestone Sep 7, 2020
@brockallen brockallen self-assigned this Sep 7, 2020
@brockallen brockallen mentioned this issue Sep 8, 2020
@brockallen
Copy link
Member

Yes, you're absolutely right -- I made the change on my docs PR. Thank you!

@leastprivilege leastprivilege removed this from the 4.1.0 milestone Sep 14, 2020
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants