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

Pass req to context enrichers #114

Open
LukasHeimann opened this issue Jan 26, 2023 · 6 comments
Open

Pass req to context enrichers #114

LukasHeimann opened this issue Jan 26, 2023 · 6 comments
Labels
enhancement New feature or request pinned

Comments

@LukasHeimann
Copy link

LukasHeimann commented Jan 26, 2023

Describe the feature request

#87 introduced the notion of ContextEnrichers, which take the context of feature toggle evaluation modify it in some meaningful way. The use case of #87 is built around using fields that are already part of the context from the start.

To support more use cases, it would be helpful to pass the full request to context enrichers, so further custom information can be pulled from it into the context.

Background

Taking only fields from the existing context to enrich it is really limiting. This is especially true if you run the unleash-proxy embedded in a node application that already preprocesses the request a bit. Imagine running an application with authentication and having session information at hand (at req.user).

You would want to provide the user information as part of the context to use it in strategies. Without having access to the request, this is not possible. A workaround could only be knowing which requests unleash-proxy handles and how the context is transferred in each of them (req.query, req.body), and adding the fields there via a custom middleware, completely sidestepping the context enrichers in an honestly rather fragile way.

Solution suggestions

Enhance the ContextEnrichter interface to consume a Request. Pass the current req to the enrichContext function, so it can provide it to the context enricher chain.

This depends partly on #109, which makes sure enrichContext is actually called consistently across use endpoints

@LukasHeimann LukasHeimann added the enhancement New feature or request label Jan 26, 2023
@sighphyre
Copy link
Member

Hey @LukasHeimann, yeah I think this makes a lot of sense, I'm very open to this idea. We probably won't make a plan to look into this this quarter but we're very open to pull requests if you or someone else has some ideas here.

I'll poke the author of the linked PR and see if we can get that merged

@nunogois nunogois moved this from New to Todo in Issues and PRs Feb 6, 2023
@LukasHeimann
Copy link
Author

Hi @sighphyre and @ivarconr,

is there any update on #109? It just went stale again...

@ivarconr
Copy link
Member

#109 if fixed and included in v0.15.

I still think it is relevant to consider how context enriches could have access to the request object, to make use of this to enrich the context.

@LukasHeimann
Copy link
Author

That's great! I'll see when I find time to provide a PR for this. If someone else wants to have a go at this in the meantime, I'd be happy as well.

@ivarconr
Copy link
Member

I was thinking; maybe we should not provide the entire req object, which is tied to express, but instead relevant parts such as headers, the user IP address and other relevant parts?

@LukasHeimann
Copy link
Author

In my particular case, I use the proxy as a middleware only, and there are non-standard things attached to the request that I'd need to know to enrich the context

@ivarconr ivarconr moved this from Todo to For later in Issues and PRs May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pinned
Projects
Status: For later
Development

No branches or pull requests

3 participants