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

[Bug] Avoid IHttpContextAccessor as much as possible. #248

Closed
jmprieur opened this issue Jun 24, 2020 · 4 comments
Closed

[Bug] Avoid IHttpContextAccessor as much as possible. #248

jmprieur opened this issue Jun 24, 2020 · 4 comments

Comments

@jmprieur
Copy link
Collaborator

jmprieur commented Jun 24, 2020

Which Version of Microsoft Identity Web are you using ?
Microsoft Identity Web 0.1.5-preview

Repro
Avoid IHttpContextAccessor as much as possible:

  • It gives a false sense of security. Everywhere where we can access the HttpContext directly we should do so. It’s not obvious in the multi-threaded methods that they use the HttpContext (which is not thread safe). Hiding that we use it leads to unsafe use.
  • A lot of time, the HttpContext is passed to access one field. There is a balance between the simple developer experience and
    the safety. Secretly accessing the HttpContext is not on the right side of the balance. For instance:
    ReplyForbiddenWithWwwAuthenticateHeader should pass in the HttpContext rather than relying on the accessor, it's called directly from a controller anyways (See TokenAcquisition.cs#L506)

requires more spec.

@jmprieur
Copy link
Collaborator Author

@jennyf19 : the work you've been doing on adding suggested cache key in the TokenSerializationArgs will remove a lot of this work

@jmprieur
Copy link
Collaborator Author

Blocked until we have the cacheKey for the token cache providers.

@jennyf19 jennyf19 self-assigned this Jul 1, 2020
@jmprieur
Copy link
Collaborator Author

Verified we've removed all what we could.

@jennyf19
Copy link
Collaborator

Included in 0.2.0-preview release

jmprieur added a commit that referenced this issue Jul 15, 2020
jennyf19 added a commit that referenced this issue Jul 20, 2020
* Related to:
- #248
- #38

* fix build warnings (#325)

* update xml

Co-authored-by: Jean-Marc Prieur <jmprieur@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants