eID-Service Integration

BenjaminKeller commented 7 years ago

This pull request implements the required functionality of the interfaces of the eID-Service.

It contains the interface to the eID-Server so the SAML-procedure has only to modify the value of restrictedId in the ServiceRequest table. @Armagetron Please give some feedback about that.

Moreover it provides the view getUserId for the openId Provider. The refreshAddress provides a json containing the access token:

{"accessToken": "9465901b-6693-462b-9044-db3a80fe0cd0"}

getUserId provides the restricted id as base64:

{"userId": "b''"}

@nielsgroth @m273d15 @larissazech Please give some feedback.

See also #44 and #46.

nils-wisiol commented 7 years ago

It may be worthwhile to add a README to the eid server Django app to summarize the interface. Alternatively, this could be entirely put in code docs.

BenjaminKeller commented 7 years ago

You're right. Doc is now required as the complexity is high. I put it in code.

Update: I added documentation.

TODO:

[x] Rewrite tests according to new framework
[x] Add negative tests
[x] Adapt interface to OpenId-Provider (see #62)

auvin commented 7 years ago

Currently the openid provider expects a POST with username, client_id, redirect_uri, response_type, scope and state. The username is used to present the restricted userID, so i think we can than use the provided getUserId for that. The other attributes are present as soon as the user clicks 'login with eid' and should be forwarded from that point on.

@larissazech, @m273d15 Please correct me, if i am wrong.

Edit: See #62 for a description more precisely.

BenjaminKeller commented 7 years ago

Chakram seems to be broken, too? @nils-wisiol Done.

nils-wisiol commented 7 years ago

@BenjaminKeller

Chakram seems to be broken, too?

looks good to me, why do you think it is broken?

BenjaminKeller commented 7 years ago

@nils-wisiol The link is not working. Have a look at build #266 in travis. the tc token generator returns a tc token is the only test that fails but it affects 6 other tests, too.

Rebasing done.

nils-wisiol commented 7 years ago

In the travis build, tests fail as regex do not match. The Location that was sent,

https://eid.local/api/openid/login?eid_access_token=5cd623f4-9cd4-46a4-b49c-ea330fe8fb11&test=bla&foo=bar

does indeed not match the given expression

/\/api\/openid\/login?eid_access_token=.*[&]test=bla[&]foo=bar$/

Try escaping ? and _:

/\/api\/openid\/login\?eid\_access\_token=.*[&]test=bla[&]foo=bar$/

Also useful.

BenjaminKeller commented 7 years ago

Hi @nils-wisiol your described issue was problem of the test The refresh page redirects correctly with GET parameters. And obviously it is now working and I have fixed that a day ago. The other tests (eID-Service: The tcTokenUrl generator redirects by default and so on) are in a completely different file with a completely different definition. The regex in this file is also completely different from the displayed. And they fail because the the other test fails. That's the problem.

Nevertheless, this branch can be merged.

nils-wisiol commented 7 years ago

I'd blame that problem on the double-definition (here and here) of the correctRedirect property.

Chakram (using chai) runs all tests parallel, so you get a race condition and your expectation was overwritten.

I didn't reproduce this locally, but unless there is indication that this theory could be wrong, I'd rather spare the work.

BenjaminKeller commented 7 years ago

This shouldn't be a problem as every suite has its own before call.

Do you have any further suggestions or can we merge?

nils-wisiol commented 7 years ago

The double definition is put onto a singleton object, that's why the old one is overwritten. (That's how require.js works)

BenjaminKeller commented 7 years ago

@nils-wisiol Files renamed, database migrated and tests improved. Do you check line feeds, formatting and style conventions manually or do you have some cool tool?

nils-wisiol commented 7 years ago

I do not use a tool to check style conventions on reviews (unless I have a suspicion), but I recommend pycharm's integrated style checker. For line endings, I find GNU/Linux' file command helpful. I used it for checking line endings in a bunch of files at once.

BenjaminKeller commented 7 years ago

@nils-wisiol Thank you very much for your answer. I fixed the typo. I will use a constant redirect test function for testing next time!

nils-wisiol commented 7 years ago

Alrighty. Looking forward to see this in master.

On August 29, 2017 10:49:52 PM GMT+02:00, BenjaminKeller notifications@github.com wrote:

@nils-wisiol Thank you very much for your answer. I fixed the typo. I will use a constant redirect test function for testing next time!

-- 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/57#issuecomment-325799743

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

swp-fu-eid / eid-fu-swp

eID-Service Integration #57