Morning, answers inline.

--
abstractj

On January 22, 2014 at 8:26:25 AM, Sebastien Blanc (scm.blanc@gmail.com) wrote:
>
> Hi,
> Since the User Management PR has been submitted [1] there has
> been of a lot
> of useful and interesting feedback, thanks Matzew and Abstractj
> for that.
>
> Some valid concerns has been raised, in particular 2 of them that
> I would
> like to expose here and to discuss to see how we can deal witth them
> for
> the 0.10.0 release :
>
> 1.
> Currently the password register/reset link that is generated
> is persisted.
> This is a point of concern[2]. The fact is that with the current
> flow, we
> can not go against that:
>
> - An admin create an user, a link is generated.
> - The admin send this link to the new user.
> - The user browse to link -> at this moment we need to be able to retrieve
> the stored link to check for its validity.

I completely understand that you guys need to release it by the end of this month and I’m not against it. The goal was just to alert about the risks and maybe create new Jiras for it (only to people know that we are aware of it).

>
> Some points :
> - The token/register link is presisted without any relation
> with the newly
> created user, so an hacker could not make a connection between
> the 2.

Maybe is my misunderstanding about the big picture, but the token has no relation with the newly created user, why do we have something like this? https://github.com/aerogear/aerogear-unifiedpush-server/pull/118#discussion-diff-8996740R29

> - The new created user, as long he has not registered through the
> link,
> can not log into the system as he has no password, as Bruno suggested
> me to
> do on the ML.
>
> How shall we deal with that for 0.10.0 ? We can improve in 0.11.0 

If we are aware of the improvements to the next release, why not?

Just make sure to remove any relation between users and tokens (at least for password reseting) and also make it clear to the “admin” that with "great power comes with great responsibility”.

I would suggest to point them to Jiras at our docs, we can always improve. 

> and also
> keeping in mind that keycloak could come into the party quite
> soon.

We need to keep our eyes in Keycloak, but they are still under development, the best approach is to make clear our needs, ideas and send some code to the project, IMO.

>
> 2.
> Currently, to generate the register link, we use a private key.
> This key is
> located in the project[3]. This should not be in the github project
> as
> pointed by Bruno [4] which make totally sense. I will remove the
> private
> key from the repo and add instructions to tell how and where to
> put your
> private key.
>
> But I don't know how to deal for the UPS cartdridge, since we ship
> a war,
> the private key will be missing. Any ideas input on that is welcome. 

I can help on it and change my priorities if necessary, just not sure about the timing for you guys (I can do it today and make it available tomorrow for testing).

Some ideas:

- Automagically generate the property file with the secret key, into this way that would be per application.
- If we don’t have too much time, provide a script with the steps to upload the keys on OpenShift (maybe the first step is less confuse)

> For 0.10.0, we could just ship a war containing a private key and
> add a
> warning and maybe add instruction on how to clone the app locally,
> change
> the key and push again (which is not really user friendly when
> you expect
> to have a cartdridge that "just work”).

+1 in the worst case scenario

>
> Notice that there is ticket to be able to manage your private key
> from the
> Admin console[5]

That would be nice

>
> Again, in the future, keycloak could be used also to manage the
> keys.
>
> Besides that, the current PR, in terms of functionnalities works
> : you can
> create and manage users like specified in the specs.
>
> So for the (very soon) 0.10.0 release how shall we deal with these
> concerns
> ?

Looking at UPS roadmap (https://issues.jboss.org/browse/AGPUSH) the date for 0.10.0 is Wed, right? I think independent of our decision now, the timing is too tight to merge your PR and release it tomorrow, maybe we should release it on the next week? Or do not merge that PR?

>
> Sebi
>
>
>
> [1] https://github.com/aerogear/aerogear-unifiedpush-server/pull/118
> [2]
> https://github.com/aerogear/aerogear-unifiedpush-server/pull/118#discussion_r8996725
> [3]
> https://github.com/aerogear/aerogear-unifiedpush-server/blob/register_link/src/main/resources/META-INF/config.properties
> [4]
> https://github.com/aerogear/aerogear-unifiedpush-server/pull/118#discussion_r8996712
> [5] https://issues.jboss.org/browse/AGPUSH-518

> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev


_______________________________________________
aerogear-dev mailing list
aerogear-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/aerogear-dev