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

Modify expand_login_view to allow for subdomain and host matching for login_view #462

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

travispaxton
Copy link
Contributor

The current implementation will throw the following error when using dynamic subdomain or host matching in Flask:

werkzeug.routing.BuildError: Could not build url for endpoint 'login'. Did you forget to specify values ['tenant_name']?

This change will allow for the following:

login_manager.login_view = 'login'


@app.route('/', subdomain='<tenant_name>')
@login_required
def dashboard(tenant_name):
    ...


@app.route('/login', methods=['GET', 'POST'], subdomain='<tenant_name>')
def login(tenant_name):
    ...

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 425a839 on travispaxton:master into 5fce013 on maxcountryman:master.

@maxcountryman maxcountryman merged commit ade6bbd into maxcountryman:master Feb 12, 2020
luyizhao added a commit to luyizhao/flask-login that referenced this pull request Aug 2, 2021
…in special case

In maxcountryman#462, the requested view arguments were passed to the `login_view` so as to allow for the use of dynamic subdomains in Flask. However, this had the side effect of passing any requested view arguments to the `login_view`. This is unnecessary and may also lead to unanticipated side effects if some arguments inadvertently passed take on special meaning in the `login_view`.

Instead, special case dynamic subdomain view arguments only so that they are the only ones passed to `login_view`.
luyizhao added a commit to luyizhao/flask-login that referenced this pull request Aug 2, 2021
…in special case

In maxcountryman#462, the requested view arguments were passed to the `login_view` so as to allow for the use of dynamic subdomains in Flask. However, this had the side effect of passing any requested view arguments to the `login_view`. This is unnecessary and may also lead to unanticipated side effects if some arguments inadvertently passed take on special meaning in the `login_view`.

Instead, special case dynamic subdomain view arguments only so that they are the only ones passed to `login_view`.
luyizhao added a commit to luyizhao/flask-login that referenced this pull request Aug 2, 2021
…in special case

In maxcountryman#462, the requested view arguments were passed to the `login_view` so as to allow for the use of dynamic subdomains in Flask. However, this had the side effect of passing any requested view arguments to the `login_view`. This is unnecessary and may also lead to unanticipated side effects if some arguments inadvertently passed take on special meaning in the `login_view`.

Instead, special case dynamic subdomain view arguments only so that they are the only ones passed to `login_view`.
luyizhao added a commit to luyizhao/flask-login that referenced this pull request Aug 2, 2021
…in special case

In maxcountryman#462, the requested view arguments were passed to the `login_view` so as to allow for the use of dynamic subdomains in Flask. However, this had the side effect of passing any requested view arguments to the `login_view`. This is unnecessary and may also lead to unanticipated side effects if some arguments inadvertently passed take on special meaning in the `login_view`.

Instead, special case dynamic subdomain view arguments only so that they are the only ones passed to `login_view`.
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2022
@davidism
Copy link
Collaborator

I don't think this should have been merged. For some "global" variables in URL rules, like multi-tennant, locale, etc., you should use app.url_value_preprocessor and app.url_defaults to extract to g at the beginning of the request and inject in url building.

This change added all URL values as query args to the login view, not just subdomain and host. #663 addressed that, but had to use an internal Werkzeug API to inspect the URL rule each time (overhead), and was broken when it was removed from Werkzeug.

@app.url_value_preprocessor
def store_tennant(endpoint, values):
    g.tennant = values.pop("tennant", None)

@app.url_defaults
def set_tennant(endpoint, values):
    values.setdefault("tennant", g.tennant)

@maxcountryman
Copy link
Owner

Agree. This seems like the wrong approach. We should revert this.

@davidism
Copy link
Collaborator

I'll revert it and release 0.6.2.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants