Fixes #1774 Functional tests for OIDC authentication

[tleyden]

This change is

[tleyden]

This includes fix for #1774

Jenkins functional test in progress:

http://uberjenkins.sc.couchbase.com:8080/view/QE%20Dev/job/syncgateway-functional-tests-dev/124/

[tleyden]

@ajres included you as part of the PR review, we should plan to do a screenshare session with the three of us.

[tleyden]

@seth I pushed up the changes we discussed and rebased off current master.

Functional test re-run in progress:

http://uberjenkins.sc.couchbase.com:8080/view/QE%20Dev/job/syncgateway-functional-tests-dev/132/console

[sethrosetter]

Reviewed 4 of 7 files at r1, 1 of 1 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

testsuites/syncgateway/functional/test_openid_connect.py, line 88 [r3] (raw file):

    www_auth_header = response.headers['Www-Authenticate']
    max_split = 1
    oidc_login_url = www_auth_header.split("=", max_split)[1]              # '"http://localhost:4984/db/_oid ..."'

This comment is confusing

---

testsuites/syncgateway/functional/test_openid_connect.py, line 147 [r3] (raw file):

    # make a request using the ID token against the db and expect a 200 response
    headers = {"Authorization": "Bearer {}".format(id_token)}
    db_url = "{}/{}".format(sg_url, sg_db)

Can we assume that if we have authorization to the db endpoint, that all other endpoints will function as expected?

---

testsuites/syncgateway/functional/test_openid_connect.py, line 177 [r3] (raw file):

        # make sure we get a unique id token each time
        assert id_token_refresh not in id_tokens
        id_tokens.append(id_token_refresh)

Would it be a good assertion to test that the new id tokens can also communicate with the live sync gateway (i.e. GET /db)?

---

testsuites/syncgateway/functional/test_openid_connect.py, line 262 [r3] (raw file):

    id_token = response_json["id_token"]

    time.sleep(token_expiry_seconds * 2)

Why * 2, should a delta of + 1 be sufficient?

---

testsuites/syncgateway/functional/test_openid_connect.py, line 319 [r3] (raw file):

    # add a garbage last component
    all_components_except_last.append("garbage")

Should this be more realistic the last piece of the token, (i.e. expected length) or do you think this is fine?

---

testsuites/syncgateway/functional/test_openid_connect.py, line 408 [r3] (raw file):

    log_info("decoded_id_token: {}".format(decoded_id_token))

    assert "nickname"  in decoded_id_token.keys()

Is more than "nickname" expected in a large scope or should we assert others as well?

---

testsuites/syncgateway/functional/1sg_1cbs/1sg_1cbs-openid-connect.robot, line 57 [r3] (raw file):

    ...  and make sure the token is rejected
    [Tags]   sanity
    Test OpenIDConnect Expired Token  sg_url=${sg_url}  sg_db=${sg_db}

What is the behavior for a ttl on the token for the past?

---

Comments from Reviewable

[tleyden]

Review status: 4 of 7 files reviewed at latest revision, 7 unresolved discussions.

---

testsuites/syncgateway/functional/test_openid_connect.py, line 88 [r3] (raw file):

Previously, sethrosetter (Seth Rosetter) wrote…

This comment is confusing

Done.

---

testsuites/syncgateway/functional/test_openid_connect.py, line 147 [r3] (raw file):

Previously, sethrosetter (Seth Rosetter) wrote…

Can we assume that if we have authorization to the db endpoint, that all other endpoints will function as expected?

Not sure I understand. Are you saying that the test might be weak in the sense that it only validates one endpoint?

---

testsuites/syncgateway/functional/test_openid_connect.py, line 177 [r3] (raw file):

Previously, sethrosetter (Seth Rosetter) wrote…

Would it be a good assertion to test that the new id tokens can also communicate with the live sync gateway (i.e. GET /db)?

Done.

---

testsuites/syncgateway/functional/test_openid_connect.py, line 262 [r3] (raw file):

Previously, sethrosetter (Seth Rosetter) wrote…

Why * 2, should a delta of + 1 be sufficient?

Done.

---

testsuites/syncgateway/functional/test_openid_connect.py, line 319 [r3] (raw file):

Previously, sethrosetter (Seth Rosetter) wrote…

Should this be more realistic the last piece of the token, (i.e. expected length) or do you think this is fine?

I think it's fine since I can't envision a scenario where it would be let through due to being garbage of the same length

---

testsuites/syncgateway/functional/test_openid_connect.py, line 408 [r3] (raw file):

Previously, sethrosetter (Seth Rosetter) wrote…

Is more than "nickname" expected in a large scope or should we assert others as well?

Nickname is the only one I am aware of. I think it's sufficient.

---

testsuites/syncgateway/functional/1sg_1cbs/1sg_1cbs-openid-connect.robot, line 57 [r3] (raw file):

Previously, sethrosetter (Seth Rosetter) wrote…

What is the behavior for a ttl on the token for the past?

Good catch! I added a new test for that which is currently passing

---

Comments from Reviewable

[sethrosetter]

Reviewed 3 of 3 files at r4.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

testsuites/syncgateway/functional/test_openid_connect.py, line 147 [r3] (raw file):

Previously, tleyden (Traun Leyden) wrote…

Not sure I understand. Are you saying that the test might be weak in the sense that it only validates one endpoint?

That is correct. Do we need to validate against other sync_gateway endpoints or can we assume that if we have access to the database endpoint that the access for the other endpoints we expect to be able to query will will work as expected.?

---

Comments from Reviewable