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

Reimplement has_access decorator. #2028

Merged
merged 1 commit into from
Jan 26, 2017
Merged

Reimplement has_access decorator. #2028

merged 1 commit into from
Jan 26, 2017

Conversation

bkyryliuk
Copy link
Member

@bkyryliuk bkyryliuk commented Jan 24, 2017

I moved has_access to utils and extended the redirect upon successful login to provide next arg that can be used by the login implementation to redirect user back to the original path.

I plan to contribute it back to the FAB later on.

Partially solves: #1303

It can be used then in the login implementation

from flask import request, url_for

def redirect_url():
    return request.args.get('next') or request.referrer or url_for('index')


class AuthRemoteUserView(AuthRemoteUserView):
    @expose('/login/')
    def login(self):
      .....
      if success:
         return redirect(self.redirect_url())

Additional links:

Reviewers:

FLAMSG_ERR_SEC_ACCESS_DENIED,
PERMISSION_PREFIX
)
from flask_appbuilder._compat import as_unicode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this? seems like it could bite us in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mistercrunch - I just copied existing implementation:
https://github.com/dpgaspar/Flask-AppBuilder/blob/af095ac4094d068fa2dd467bd01fa8b95d697fc1/flask_appbuilder/security/decorators.py#L5

I plan to create another PR to path FAB is possible, there is a TODO for it.

@bkyryliuk bkyryliuk merged commit 1ac2273 into master Jan 26, 2017
@bkyryliuk bkyryliuk deleted the fix_login_redirects branch January 26, 2017 20:13
SalehHindi pushed a commit to SalehHindi/superset that referenced this pull request Jun 9, 2017
@rpwils
Copy link

rpwils commented Nov 6, 2017

Has there been anymore progress on this issue?

@rpwils
Copy link

rpwils commented Nov 9, 2017

xrmx - This solution was not fully implemented in superset. The auth views in flask appbuilder do not take this change into account and they are not implemented in superset. We have implemented oauth login without the user needing to click the login buttons. When this occurs there is a popup to warn the user that Access was denied which is implemented in the has_access method thus forcing us to over write both. Any thought on how to resolve this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants