Projet

Général

Profil

0001-auth_oidc-dont-stop-redirect-on-state-lost-47825.patch

Benjamin Dauvergne, 27 octobre 2020 17:09

Télécharger (7,6 ko)

Voir les différences: 

Subject: [PATCH 1/2] auth_oidc: dont stop redirect on state lost (#47825)

Next url is usually to a local view requiring login, so on state lost we
can just continue to original next_url and it will start a new login.

 src/authentic2_auth_oidc/views.py | 15 +++++++-----
 tests/test_auth_oidc.py           | 40 ++++++++++++++++++++++++++-----
 2 files changed, 43 insertions(+), 12 deletions(-)
src/authentic2_auth_oidc/views.py
41 41 
    provider = get_provider(pk)
42 42 
    scopes = set(provider.scopes.split()) | set(['openid'])
43 43 
    state = str(uuid.uuid4())
44 
    next_url = next_url or request.GET.get(REDIRECT_FIELD_NAME, '')
45 
    if next_url and not good_next_url(request, next_url):
46 
        next_url = None
44 47 
    nonce = request.GET.get('nonce') or str(uuid.uuid4())
45 48 
    display = set()
46 49 
    prompt = set()
......
49 52 
        'scope': ' '.join(scopes),
50 53 
        'response_type': 'code',
51 54 
        'redirect_uri': request.build_absolute_uri(reverse('oidc-login-callback')),
52 
        'state': state,
55 
        'state': state if not next_url else state + ' ' + next_url,
53 56 
        'nonce': nonce,
54 57 
    }
55 58 
    if provider.claims_parameter_supported:
......
73 76 
    saved_state = request.session.setdefault('auth_oidc', {}).setdefault(state, {})
74 77 
    saved_state['request'] = params
75 78 
    saved_state['issuer'] = provider.issuer
76 
    next_url = next_url or request.GET.get(REDIRECT_FIELD_NAME, '')
77 
    if good_next_url(request, next_url):
79 
    if next_url:
78 80 
        saved_state['next_url'] = next_url
79 81 
    request.session.modified = True  # necessary if auth_oidc already exists
80 82 
    logger.debug('auth_oidc: sent request to authorization endpoint %r', params)
......
102 104 
    def get(self, request, *args, **kwargs):
103 105 
        logger = logging.getLogger(__name__)
104 106 
        code = request.GET.get('code')
105 
        state = request.GET.get('state')
107 
        raw_state = request.GET.get('state', '')
108 
        state = raw_state.split(' ', 1)[0]
106 109 
        oidc_state = self.oidc_state = request.session.get('auth_oidc', {}).get(state)
107 110 
        if not state or not oidc_state or 'request' not in oidc_state:
108 
            messages.warning(request, _('Login with OpenIDConnect failed, state lost.'))
109 111 
            logger.warning('auth_oidc: state lost')
110 
            return redirect(request, settings.LOGIN_REDIRECT_URL)
112 
            state_next_url = raw_state.split(' ', 1)[1] if ' ' in raw_state else None
113 
            return redirect(request, state_next_url or settings.LOGIN_REDIRECT_URL)
111 114 
        oidc_request = oidc_state.get('request')
112 115 
        assert isinstance(oidc_request, dict), 'state is not properly initialized'
113 116 
        nonce = oidc_request.get('nonce')
tests/test_auth_oidc.py
496 496 
    assert location.netloc == endpoint.netloc
497 497 
    assert location.path == endpoint.path
498 498 
    query = check_simple_qs(urlparse.parse_qs(location.query))
499 
    assert query['state'] in app.session['auth_oidc']
499 
    state = query['state'].split()[0]
500 
    assert state in app.session['auth_oidc']
500 501 
    assert query['response_type'] == 'code'
501 502 
    assert query['client_id'] == str(oidc_provider.client_id)
502 503 
    assert query['scope'] == 'openid'
503 504 
    assert query['redirect_uri'] == 'http://testserver' + reverse('oidc-login-callback')
504 505 
    # get the nonce
505 
    nonce = app.session['auth_oidc'][query['state']]['request']['nonce']
506 
    nonce = app.session['auth_oidc'][state]['request']['nonce']
506 507 

  		




  507
  508
  
    
    if oidc_provider.claims_parameter_supported:

  		




  508
  509
  
    
        claims = json.loads(query['claims'])

  		




  ......




  627
  628
  
    
    response = response.click(oidc_provider.name)

  		




  628
  629
  
    
    location = urlparse.urlparse(response.location)

  		




  629
  630
  
    
    query = check_simple_qs(urlparse.parse_qs(location.query))

  		




  630
  
  
    
    nonce = app.session['auth_oidc'][query['state']]['request']['nonce']

  		




  
  631
  
    
    state = query['state'].split()[0]

  		




  
  632
  
    
    nonce = app.session['auth_oidc'][state]['request']['nonce']

  		




  631
  633
  
    

  		




  632
  634
  
    
    # sub=john.doe, MUST not work

  		




  633
  635
  
    
    with utils.check_log(caplog, 'cannot create user'):

  		




  ......




  669
  671
  
    
    response = response.click(oidc_provider.name)

  		




  670
  672
  
    
    location = urlparse.urlparse(response.location)

  		




  671
  673
  
    
    query = check_simple_qs(urlparse.parse_qs(location.query))

  		




  672
  
  
    
    nonce = app.session['auth_oidc'][query['state']]['request']['nonce']

  		




  
  674
  
    
    state = query['state'].split()[0]

  		




  
  675
  
    
    nonce = app.session['auth_oidc'][state]['request']['nonce']

  		




  673
  676
  
    

  		




  674
  677
  
    
    # sub=john.doe

  		




  675
  678
  
    
    with utils.check_log(caplog, 'auth_oidc: created user'):

  		




  ......




  743
  746
  
    
    response = response.click(oidc_provider_rsa.name)

  		




  744
  747
  
    
    location = urlparse.urlparse(response.location)

  		




  745
  748
  
    
    query = check_simple_qs(urlparse.parse_qs(location.query))

  		




  746
  
  
    
    nonce = app.session['auth_oidc'][query['state']]['request']['nonce']

  		




  
  749
  
    
    state = query['state'].split()[0]

  		




  
  750
  
    
    nonce = app.session['auth_oidc'][state]['request']['nonce']

  		




  747
  751
  
    

  		




  748
  752
  
    
    # test invalid kid

  		




  749
  753
  
    
    with utils.check_log(caplog, message='not in key set', levelname='WARNING'):

  		




  ......




  808
  812
  
    
    response = response.click(oidc_provider.name)

  		




  809
  813
  
    
    location = urlparse.urlparse(response.location)

  		




  810
  814
  
    
    query = check_simple_qs(urlparse.parse_qs(location.query))

  		




  811
  
  
    
    nonce = app.session['auth_oidc'][query['state']]['request']['nonce']

  		




  
  815
  
    
    state = query['state'].split()[0]

  		




  
  816
  
    
    nonce = app.session['auth_oidc'][state]['request']['nonce']

  		




  812
  817
  
    

  		




  813
  818
  
    
    with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, nonce=nonce):

  		




  814
  819
  
    
        response = app.get(login_callback_url(oidc_provider), params={'code': code, 'state': query['state']}).maybe_follow()

  		




  ......




  822
  827
  
    
    assert user.last_name == 'DOE'

  		




  823
  828
  
    
    # typo in template string, no rendering

  		




  824
  829
  
    
    assert user.first_name == '{{ given_name'

  		




  
  830
  
    

  		




  
  831
  
    

  		




  
  832
  
    
def test_lost_state(app, caplog, code, oidc_provider, oidc_provider_jwkset, hooks):

  		




  
  833
  
    
    response = app.get('/login/?next=/whatever/')

  		




  
  834
  
    
    assert oidc_provider.name in response.text

  		




  
  835
  
    
    response = response.click(oidc_provider.name)

  		




  
  836
  
    
    qs = urlparse.parse_qs(urlparse.urlparse(response.location).query)

  		




  
  837
  
    
    state = qs['state'][0]

  		




  
  838
  
    
    assert ' /whatever/' in state

  		




  
  839
  
    
    nonce = app.session['auth_oidc'][state.split()[0]]['request']['nonce']

  		




  
  840
  
    

  		




  
  841
  
    
    # reset the session to forget the state

  		




  
  842
  
    
    app.session.flush()

  		




  
  843
  
    

  		




  
  844
  
    
    caplog.clear()

  		




  
  845
  
    
    with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, nonce=nonce):

  		




  
  846
  
    
        response = app.get(login_callback_url(oidc_provider), params={'code': code, 'state': state})

  		




  
  847
  
    
    # not logged

  		




  
  848
  
    
    assert 'auth_oidc: state lost' == caplog.records[-1].message

  		




  
  849
  
    
    # event is recorded

  		




  
  850
  
    
    assert '_auth_user_id' not in app.session

  		




  
  851
  
    
    # we are automatically redirected to our destination

  		




  
  852
  
    
    assert response.location == '/whatever/'

  		




  825
  
  
    
-