[keycloak-dev] Testing with mocking libraries?

Fri Jun 8 02:01:56 EDT 2018

Looks like a temporary connection issue as it was failing before running
any tests. I've rescheduled the tests. Can you provide some details on what
we need to run the SocialLoginTest now?

On 7 June 2018 at 21:52, Steffen Kreutz <s.kreutz at yieldlab.de> wrote:

> Yes, this must be done inside the identity provider when Google sends the
> token because attackers might intercept the request and change the hd
> parameter.
>
> Unfortunately the integration tests failed randomly on Travis without
> showing any output. In my first run it were TESTS=server-group4 and
> TESTS=old and in the second TESTS=server-group1, TESTS=server-group2 and TESTS=server-group4.
> Could this be related to my changes?
>
> Best,
>
> Steffen
>
>
> Stian Thorgersen <sthorger at redhat.com> schrieb am Do., 7. Juni 2018,
> 21:20:
>
>>
>>
>> On 7 June 2018 at 10:11, Steffen Kreutz <s.kreutz at yieldlab.de> wrote:
>>
>>> I will remove the unit tests and mocking stuff from the PR and create
>>> integration tests.
>>>
>>
>> Great, thanks
>>
>>
>>>
>>> Would it be sufficient to test if logging in succeeds for an account
>>> with a valid domain and accordingly fails for an account with an invalid
>>> domain? Or should I somehow test if the token contains the hd param.
>>>
>>
>> Your code already checks if the hd param in the token matches the
>> requested hd though doesn't it? So testing valid/invalid domain seems
>> sufficient to me.
>>
>>
>>>
>>>
>>> > Am 06.06.2018 um 16:17 schrieb Marek Posolda <mposolda at redhat.com>:
>>> >
>>> > +1 for investigate. I hope the contributor can help us with clarifying
>>> > it and write the test?
>>> >
>>> > Maybe the test for this will need to be in separate class, so it's
>>> > possible to test it separately from the other Google IDP stuff, which
>>> > can be tested with free account. And the other contributors to Google
>>> > IDP, which don't have corporate account, won't be blocked.
>>> >
>>> > But I suppose that for our CI, we can test with "redhat.com" domain
>>> account?
>>> >
>>> > Marek
>>> >
>>> >
>>> > On 06/06/18 15:22, Stian Thorgersen wrote:
>>> >> I think we're in agreement here that the ideal is to do proper
>>> >> integration tests. Rather than open the doors for mocks (java based
>>> or
>>> >> http based) we should attempt to do it proper. For Google I'm not
>>> sure
>>> >> how realistic that is as the hd param probably requires an corporate
>>> >> account rather than a freebie account that we are using today for
>>> >> testing. We should investigate that first though.
>>> >>
>>> >> On 6 June 2018 at 10:03, Hynek Mlnarik <hmlnarik at redhat.com
>>> >> <mailto:hmlnarik at redhat.com>> wrote:
>>> >>
>>> >>    To answer the original question whether to mock or not to mock, I
>>> >>    wouldn't
>>> >>    object allowing those in unit tests (which we have not that many
>>> >>    though
>>> >>    until now there's no need to them and I hope we can preserve this
>>> >>    state).
>>> >>    I'd rather not introduce those in integration tests because of
>>> >>    maintenance
>>> >>    costs.
>>> >>
>>> >>    The biggest issue I see with mocks is that one needs to be extra
>>> >>    careful to
>>> >>    return exactly what's expected from the specification/counterparty.
>>> >>    Since social
>>> >>    providers  can change without prior notice, our mocked IdPs might
>>> >>    give us
>>> >>    false impression that we're testing them. Furthermore, such mocks
>>> >>    would be
>>> >>    rather complex, and would require non-trivial maintenance. I'm
>>> >>    hence biased
>>> >>    against mock IdPs and prefer the tests with real IdPs as mentioned
>>> >>    by Marek.
>>> >>
>>> >>
>>> >>    On Mon, Jun 4, 2018 at 11:22 AM, Marek Posolda
>>> >>    <mposolda at redhat.com <mailto:mposolda at redhat.com>> wrote:
>>> >>
>>> >>> Dne 1.6.2018 v 14:48 Bill Burke napsal(a):
>>> >>>> On Fri, Jun 1, 2018 at 5:18 AM, Marek Posolda
>>> >>    <mposolda at redhat.com <mailto:mposolda at redhat.com>>
>>> >>> wrote:
>>> >>>>> For IDP, I am not sure how would Mocked IDP help? We already
>>> >>    have lots
>>> >>> of
>>> >>>>> test coverage for Identity brokering for OIDC and SAML. The
>>> >>    social
>>> >>> providers
>>> >>>>> usually implements OAuth/OIDC and there are just small
>>> >>    differences
>>> >>> between
>>> >>>>> them. But those small differences and the fact that social
>>> >>    providers
>>> >>> are out
>>> >>>>> of our control, is exactly the reason why we need to test
>>> >>    with real
>>> >>>>> providers instead of mocks IMO.
>>> >>>>>
>>> >>>> IDP mocks are not a replacement for real testing.  Just
>>> >>    something that
>>> >>>> would run in public CI, that way at least there's something to
>>> >>    catch
>>> >>>> issues like when/if brokering gets refactored again.
>>> >>> AFAIK the longer-term plan is to run social tests on every PR,
>>> >>    to catch
>>> >>> regressions by testing with real social providers. So maybe it's
>>> >>    better
>>> >>> to rather wait until we have that rather then introduce mocks now?
>>> >>>
>>> >>> Also anyone from community should be able to run social tests
>>> >>    based on
>>> >>> our README. The important is, that someone could run it just for
>>> >>    single
>>> >>> provider, which he is interested in (EG. Google for this PR) and
>>> >>    is not
>>> >>> required to create the account+application in all social providers,
>>> >>> which we support. But I hope the social tests already support
>>> >>    this now.
>>> >>>
>>> >>> Marek
>>> >>>>
>>> >>>> Bill
>>> >>>
>>> >>>
>>> >>> _______________________________________________
>>> >>> keycloak-dev mailing list
>>> >>> keycloak-dev at lists.jboss.org <mailto:keycloak-dev at lists.jboss.org>
>>> >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>> >>    <https://lists.jboss.org/mailman/listinfo/keycloak-dev>
>>> >>>
>>> >>
>>> >>
>>> >>
>>> >>    --
>>> >>
>>> >>    --Hynek
>>> >>    _______________________________________________
>>> >>    keycloak-dev mailing list
>>> >>    keycloak-dev at lists.jboss.org <mailto:keycloak-dev at lists.jboss.org>
>>> >>    https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>> >>    <https://lists.jboss.org/mailman/listinfo/keycloak-dev>
>>> >>
>>> >>
>>> >
>>> > _______________________________________________
>>> > keycloak-dev mailing list
>>> > keycloak-dev at lists.jboss.org
>>> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>
>>>
>>> _______________________________________________
>>> keycloak-dev mailing list
>>> keycloak-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>
>>
>>