Merge pull request #385 from PCMan/pcman@preserve_state_on_error_according_to_rfc6749

Bug fix: Preserve "state" parameter in the oauth2 handler when non-critical errors happen as required in RFC 6749.

Author: lepture

flask_oauthlib/provider/oauth2.py

@@ -398,6 +398,10 @@ def decorated(*args, **kwargs):
                     return redirect(e.in_uri(self.error_uri))
                 except oauth2.OAuth2Error as e:
                     log.debug('OAuth2Error: %r', e, exc_info=True)
+                    # on auth error, we should preserve state if it's present according to RFC 6749
+                    state = request.values.get('state')
+                    if state and not e.state:
+                        e.state = state  # set e.state so e.in_uri() can add the state query parameter to redirect uri
                     return redirect(e.in_uri(redirect_uri))
                 except Exception as e:
                     log.exception(e)
@@ -417,6 +421,10 @@ def decorated(*args, **kwargs):
                 return redirect(e.in_uri(self.error_uri))
             except oauth2.OAuth2Error as e:
                 log.debug('OAuth2Error: %r', e, exc_info=True)
+                # on auth error, we should preserve state if it's present according to RFC 6749
+                state = request.values.get('state')
+                if state and not e.state:
+                    e.state = state  # set e.state so e.in_uri() can add the state query parameter to redirect uri
                 return redirect(e.in_uri(redirect_uri))
 
             if not isinstance(rv, bool):
@@ -425,7 +433,7 @@ def decorated(*args, **kwargs):
 
             if not rv:
                 # denied by user
-                e = oauth2.AccessDeniedError()
+                e = oauth2.AccessDeniedError(state=request.values.get('state'))
                 return redirect(e.in_uri(redirect_uri))
             return self.confirm_authorization_request()
         return decorated
@@ -456,6 +464,10 @@ def confirm_authorization_request(self):
             return redirect(e.in_uri(self.error_uri))
         except oauth2.OAuth2Error as e:
             log.debug('OAuth2Error: %r', e, exc_info=True)
+            # on auth error, we should preserve state if it's present according to RFC 6749
+            state = request.values.get('state')
+            if state and not e.state:
+                e.state = state  # set e.state so e.in_uri() can add the state query parameter to redirect uri
             return redirect(e.in_uri(redirect_uri or self.error_uri))
         except Exception as e:
             log.exception(e)

tests/oauth2/test_oauth2.py

@@ -81,12 +81,21 @@ def test_oauth_authorize_valid_url(self):
         assert 'code=' in rv.location
         assert 'state' not in rv.location
 
-        # test state
+        # test state on access denied
+        # According to RFC 6749, state should be preserved on error response if it's present in the client request.
+        # Reference: https://tools.ietf.org/html/rfc6749#section-4.1.2
+        rv = self.client.post(authorize_url + '&state=foo', data=dict(
+            confirm='no'
+        ))
+        assert 'error=access_denied' in rv.location
+        assert 'state=foo' in rv.location
+
+        # test state on success
         rv = self.client.post(authorize_url + '&state=foo', data=dict(
             confirm='yes'
         ))
         assert 'code=' in rv.location
-        assert 'state' in rv.location
+        assert 'state=foo' in rv.location
 
     def test_http_head_oauth_authorize_valid_url(self):
         rv = self.client.head(authorize_url)