11:22 a.m.
Dne 1.6.2018 v 14:48 Bill Burke napsal(a):
On Fri, Jun 1, 2018 at 5:18 AM, Marek Posolda
<mposolda(a)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
10:03 a.m.
New subject: Testing with mocking libraries?
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(a)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(a)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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
--
--Hynek
3:22 p.m.
New subject: Testing with mocking libraries?
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(a)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(a)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(a)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(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
--
--Hynek
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
3:45 p.m.
New subject: Testing with mocking libraries?
You are right. Managing accounts with a custom domain requires a G Suite subscription
which starts at 5$/month.
Am 06.06.2018 um 15:22 schrieb Stian Thorgersen
<sthorger(a)redhat.com>:
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(a)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(a)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(a)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(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>
>
>
> --
>
> --Hynek
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
4:17 p.m.
New subject: Testing with mocking libraries?
+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(a)redhat.com
<mailto:hmlnarik@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(a)redhat.com <mailto:mposolda@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(a)redhat.com <mailto:mposolda@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(a)lists.jboss.org <mailto:keycloak-dev@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(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
<https://lists.jboss.org/mailman/listinfo/keycloak-dev>
10:11 a.m.
New subject: Testing with mocking libraries?
I will remove the unit tests and mocking stuff from the PR and create integration tests.
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.
Am 06.06.2018 um 16:17 schrieb Marek Posolda
<mposolda(a)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(a)redhat.com
> <mailto:hmlnarik@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(a)redhat.com <mailto:mposolda@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(a)redhat.com <mailto:mposolda@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(a)lists.jboss.org <mailto:keycloak-dev@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(a)lists.jboss.org <mailto:keycloak-dev@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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
12:33 p.m.
New subject: Testing with mocking libraries?
I updated the PR with the SocialLoginTest
(https://github.com/keycloak/keycloak/pull/5215/files#diff-d1c89a9cccb6539...
<https://github.com/keycloak/keycloak/pull/5215/files#diff-d1c89a9cccb6539...>).
Please tell me if this is done correctly.
Best,
Steffen
Am 07.06.2018 um 10:11 schrieb Steffen Kreutz
<s.kreutz(a)yieldlab.de>:
I will remove the unit tests and mocking stuff from the PR and create integration tests.
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.
> Am 06.06.2018 um 16:17 schrieb Marek Posolda <mposolda(a)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(a)redhat.com
>> <mailto:hmlnarik@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(a)redhat.com <mailto:mposolda@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(a)redhat.com <mailto:mposolda@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(a)lists.jboss.org <mailto:keycloak-dev@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(a)lists.jboss.org <mailto:keycloak-dev@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(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
9:20 p.m.
New subject: Testing with mocking libraries?
On 7 June 2018 at 10:11, Steffen Kreutz <s.kreutz(a)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(a)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(a)redhat.com
>> <mailto:hmlnarik@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(a)redhat.com <mailto:mposolda@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(a)redhat.com <mailto:mposolda@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(a)lists.jboss.org <mailto:keycloak-dev@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(a)lists.jboss.org <mailto:keycloak-dev@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(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
9:52 p.m.
New subject: Testing with mocking libraries?
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(a)redhat.com> schrieb am Do., 7. Juni 2018, 21:20:
On 7 June 2018 at 10:11, Steffen Kreutz <s.kreutz(a)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(a)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(a)redhat.com
> >> <mailto:hmlnarik@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(a)redhat.com <mailto:mposolda@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(a)redhat.com <mailto:mposolda@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(a)lists.jboss.org
<mailto:keycloak-dev@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(a)lists.jboss.org <mailto:keycloak-dev@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(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
8:01 a.m.
New subject: Testing with mocking libraries?
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(a)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(a)redhat.com> schrieb am Do., 7. Juni 2018,
21:20:
>
>
> On 7 June 2018 at 10:11, Steffen Kreutz <s.kreutz(a)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(a)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(a)redhat.com
>> >> <mailto:hmlnarik@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(a)redhat.com <mailto:mposolda@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(a)redhat.com <mailto:mposolda@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(a)lists.jboss.org
<mailto:keycloak-dev@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(a)lists.jboss.org
<mailto:keycloak-dev@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(a)lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>>
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>
>
8:21 a.m.
New subject: Testing with mocking libraries?
Sorry I forgot that. First you need a G Suite account with a custom domain. If you have
that you need to configure the account in social.properties. This is what I used:
common.username=steffen(a)example.com
common.password=iiQGV9xZ3muF6NK7
common.profile.firstName=Steffen
common.profile.lastName=Kreutz
common.profile.email=steffen(a)example.com
google-hosted-domain.clientId=<google client id>
google-hosted-domain.clientSecret=<google client secret>
google-hosted-domain.hostedDomain=example.com
Am 08.06.2018 um 08:01 schrieb Stian Thorgersen
<sthorger(a)redhat.com>:
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(a)yieldlab.de
<mailto:s.kreutz@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(a)redhat.com <mailto:sthorger@redhat.com>> schrieb
am Do., 7. Juni 2018, 21:20:
On 7 June 2018 at 10:11, Steffen Kreutz <s.kreutz(a)yieldlab.de
<mailto:s.kreutz@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(a)redhat.com
<mailto:mposolda@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
<http://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(a)redhat.com
<mailto:hmlnarik@redhat.com>
>> <mailto:hmlnarik@redhat.com <mailto:hmlnarik@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(a)redhat.com <mailto:mposolda@redhat.com>
<mailto:mposolda@redhat.com <mailto:mposolda@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(a)redhat.com <mailto:mposolda@redhat.com>
<mailto:mposolda@redhat.com <mailto:mposolda@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(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>
<mailto:keycloak-dev@lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>>
>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
<https://lists.jboss.org/mailman/listinfo/keycloak-dev>
>> <https://lists.jboss.org/mailman/listinfo/keycloak-dev
<https://lists.jboss.org/mailman/listinfo/keycloak-dev>>
>>>
>>
>>
>>
>> --
>>
>> --Hynek
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>
<mailto:keycloak-dev@lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>>
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
<https://lists.jboss.org/mailman/listinfo/keycloak-dev>
>> <https://lists.jboss.org/mailman/listinfo/keycloak-dev
<https://lists.jboss.org/mailman/listinfo/keycloak-dev>>
>>
>>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@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(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
<https://lists.jboss.org/mailman/listinfo/keycloak-dev>
2157
days inactive
2161
days old
10 comments
4 participants
participants (4)
-
Hynek Mlnarik -
Marek Posolda -
Steffen Kreutz -
Stian Thorgersen