Add open id provider

[auvin]

Integrated the django-oidc-provider-package to our project, which provides out of the box all the endpoints, data and logic needed to add OpenID Connect capabilities.

corresponding issue: #33

[auvin]

I dont understand which tests i should add. Currently, there are two tests for availability implemented: one for the openid-website which provides the possibility to login as admin and create clients, one for the login site showing up if a client is registered at our service and wants their users to authenticate with our service.

Furthermore, i could imagine tests of an user of a registered client who wants to login at this client with our service. But currently, we have to register the client by hand with the webinterface. Therefore, the functionality for such tests is not yet implemented.

[nils-wisiol]

@m273d15 judging from the current urls.py, there are no .well-known endpoints

[nils-wisiol]

@nielsgroth Please update the travis script to create such a client and then update the e2e tests to see if a client can authenticate against the service

[m273d15]

@nils-wisiol The described endpoints are configured in the urls.py of the provider plugin

[auvin]

I updated the travis script with some python scripts to create a django user and register a client at the provider. After running the test container both is be removed.

I am not able to implement the tests and refering to #55, i think i should interrupt my attempts with the current testing framework.

[auvin]

Now i added chakram tests for the oidc-urls and the authorization process with creating a sample clilent. @nils-wisiol Please review again.

[auvin]

I rebased to master, but i still have some trouble with chakram.

test-e2e_1 | 1) Ngnix/django provides an admin page: test-e2e_1 | AssertionError: expected status code 301 to equal 200 test-e2e_1 |
test-e2e_1 | test-e2e_1 | 2) OpenId Provider provides an openid-configuration with provider information: test-e2e_1 | AssertionError: expected status code 301 to equal 200

Can anyone help me with these chakram errors? The main_spec.js should be the same as in the master, but here it throws an error.

The other error comes from:

+        it("with provider information", function() {
+            var response = chakram.get(URL + '/.well-known/openid-configuration');
+            expect(response).to.have.json('issuer', URL);
+            expect(response).to.have.json('authorization_endpoint', URL + '/authorize');
+            expect(response).to.have.json('jwks_uri', URL + '/jwks');
+            return chakram.wait();
+        }); 

But if i write every expect independent like: return expect(response).to.have.json('issuer', URL);, there is no error.

[BenjaminKeller]

@nielsgroth Try: rebase master. Have you added extra_hosts to your test.yml?

[auvin]

I already rebased to master. Yes, i added $BOILERPLATE_DOMAIN:$BOILERPLATE_IPV4_16PREFIX.0.128 as extra host, otherwise all tests will fail.

[nils-wisiol]

setup.js sets followRedirect: false. The test engine (chai) however runs all tests simultaneously, hence you get a race condition. If followRedirect is false, then 301 != 200 is your test result. If it is true, then chakram (that is, requests.js) will follow and obtain a status 200.

[nils-wisiol]

Refer to the boilerplate backport of chakram, where I encapsulated chakram, so that all setup will be finished before tests run.

[auvin]

Okay, thanks for the explanation. So my solution is to just follow the redirect and expect the required JSON.

[nils-wisiol]

Alternatively, change the URL in a way such that no extra redirect is needed

[auvin]

Actually, i already did that. I make a get on http://eid.localhost/api/openid/.well-known/openid-configuration. There is no redirect..

However, with followRedirect: true all checks have passed.

[nils-wisiol]

Notice that Django sometimes adds/removes trailing slashes with redirects automatically

On August 29, 2017 8:14:06 PM GMT+02:00, nielsgroth notifications@github.com wrote:

Actually, i already did that. I make a get on http://eid.localhost/api/openid/.well-known/openid-configuration. There is no redirect..

However, with followRedirect: true all checks have passed.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/swp-fu-eid/eid-fu-swp/pull/41#issuecomment-325748965

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[auvin]

So, can we merge?

Never mind, i just noticed that my tests are not working properly..

[auvin]

Accordingly to the demand of @nils-wisiol of e2e tests, i need to provide a working workflow for login in with the openid-provider. Therefore i need the templates and these tests. Of course the workflow should and will be updated to match #62.

But this is the goal of the branch adjust_open_id_provider_login #48, which of course depends on this branch and will delete the login via username and password and update the tests.

[nils-wisiol]

Not sure if I understand correctly, but I agree with @BenjaminKeller that tests should only be added for features that are added with the same PR. If you add tests for features that are already in master (but untested), please do so with a separate 'bug fix' commit, so we can see which tests/features belong together.

If the templates are only needed for the e2e tests, then this needs to be refactored.

[auvin]

Currently i am adding features (and tests) which will be overwritten by #48.

EDIT: Actually, i am adding OpenID Connect capabilities as described in the first post. If i understand it correctly, i should test the authentication process with this new feature. Therefore i need an authentication process which uses the openid-provider. This process is the login with username and password and this is what i am testing right now. But as described before, #48 will update/overwrite this process because we forbid login with username and password and provide login via eid. (#62)

[auvin]

I assume you did not register a testclient beforehand?

Anyways, @larissazech, @BenjaminKeller and i decided to simplify things and we are going to work with a common branch (add_openid_provider_eid_interface) which will contain the openid provider and the adjusted login process.

We already started cherry-picking the to branches. Also these tests can be used in the new branch and we will then consider your requested changes. We thus achieve that we won't implement features and tests which will be overwritten either way and we think this saves us more time than it actually costs.

Therefore, this PR is closed.