Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register in a contest with an already existing user account #1187

Closed
MaGaroo opened this issue Aug 2, 2021 · 8 comments
Closed

Register in a contest with an already existing user account #1187

MaGaroo opened this issue Aug 2, 2021 · 8 comments

Comments

@MaGaroo
Copy link
Contributor

MaGaroo commented Aug 2, 2021

Is your feature request related to a problem? Please describe.
A user that already has an account can't register with that account in a contest(which allows registration).

Describe the solution you'd like
When someone tries to login to a contest(which allows registration) with an existing username/password create and save its Participation instance automatically.

Describe alternatives you've considered
The user can create a new account to register in that contest already; But I don't think if it's a clean way!

Additional context
Note that in case of multicontest mode, the user maybe already logged in so it's not enough to apply changes just in LoginHandler.

If this feature would be considered to be added I'm ready to implement it. Just let me know if there's any consideration about code and technical stuff!

@andreyv
Copy link
Member

andreyv commented Aug 8, 2021

Hi,

Thanks for the proposal. I've been thinking about it for some time.

Observe that the user's team is stored in Participation. Thus we need to set the team in the new Participation as well. Could we copy it from an existing Participation? Not really: there might be multiple existing participations with different teams, and the user might want to select another team anyway.

Therefore, when registering for a contest with an existing account, we still need to ask the user for the team, meaning that we need to have some HTML form to do it.

My suggestion is to re-use the registration form for this task, since it already has the necessary fields. That is, instead of adding a participation during login, leave the login page for authentication only, and let the user register for the contest using the same, but now extended, registration page.

For example, the registration form could have new radio buttons at the top:

(x) New user    ( ) New participation

which would enable/disable the form fields accordingly.

On the server side, the registration handler could do something like

if new_user:
    <add user>
else:
    <check password>

<add participation>

Additionally, with this approach the login handler doesn't ever change the database state, which, I think, is good.

Of course, this solution requires more work and may have its own quirks, but overall it looks logical and more isolated to me.

What are your thoughts on this?

@wil93
Copy link
Member

wil93 commented Aug 8, 2021

Another solution could be the following:

When ContestWebServer is started in the multi-contest setting, creating an account does not automatically create a participation. There would be an extra step before opening a contest where you are asked to "join" the contest (i.e. creating a participation for it). In the contest list we could show which contests you have already joined (e.g. by using a different color, or a specific icon for it).

In order to do this, however, we would need to change the way cookie work. Right now, we create one cookie for each contest you participate in, and this means that you could be logged-out in Contest A, but logged-in in Contest B. We should change this so that there is always only one cookie, and if you log in then you will automatically appear as logged-in in all the contests that you "joined".

@MaGaroo
Copy link
Contributor Author

MaGaroo commented Aug 8, 2021

Hi,

@andreyv I think the solution you wrote about is ok. It can realize this use case with minimum amount of changes and also most of the changes would affect UI instead of python codes.

@wil93 I guess it's possible to do the work in this way too. However there are some major changes, so deciding about it is a higher level decision and I can't easily comment about it.
Although I don't exactly know why the authentication system's been implemented in this "per contest" manner, at first it seemed a bit odd to me to be able to login in different parts of a website with different accounts.
Just to note I think separating the logic of "creating a new account" and "joining a contest" would be a clean approach, at least in the multi-contest mode as you mentioned too.

@andreyv
Copy link
Member

andreyv commented Aug 10, 2021

The main reason login and registration are implemented like this is because CMS at first supported only one contest at a time, and, while multi-contest mode was implemented later, every individual contest remained independent from other contests. In the end, this has the benefit of a simpler implementation to support both single contest and multi-contest modes, because the per-contest code can be the same for both modes.

@wil93 suggestion is also nice (thanks!), although, as you noted, it needs more extensive changes. There are some considerations too:

  • There would be two registration steps instead of one for the first contest
  • There would effectively be a state when the user is already logged in, but has no participation, complicating authentication

Therefore at this time I am inclined towards the simpler solution described above.

@wil93
Copy link
Member

wil93 commented Aug 10, 2021

Looking back, I feel that when we implemented multi-contest mode in #594 we used the wrong approach.

We tried to keep full backwards-compatibility with the single contest mode (e.g. the URL in single-contest mode does not contain the contest name, just like before) but I don't think that was a good idea.

Of course, hindsight is 20/20, but I think a better approach would have been to make multi-contest the only possible mode, still keeping the -c flag but simply using it as a "filter" to only select 1 possible contest, and possibly also having a logic that automatically adds a participation for that contest upon signup.

This approach would imply that:

  • We consistently show the list of contests on the / path, and never the actual contest page (the only caveat would be that when -c is specified, the list would contain only one element).
  • The contest path would always be /<contest_name>/, instead of /, even when -c is specified.

With this approach we could have avoided having 2*N cookies and instead kept this number down at 2.

As I said, hindsight is 20/20, but maybe it's worth going in this direction now, instead of going further with a suboptimal approach.


And just to answer these points:

There would be two registration steps instead of one for the first contest

The second step would simply consist of a "Join" button. The user (already logged in) simply needs to click that, and the participation is created.

Moreover, if -c is specified, we can automatically create the participation when signing up.

There would effectively be a state when the user is already logged in, but has no participation, complicating authentication

Yes but doing so would also lower the amount of cookies needed (from 2*N to 2), so if we make this the default behavior I think that we would simplify things, overall.

@MaGaroo
Copy link
Contributor Author

MaGaroo commented Aug 10, 2021

Just to note(not as a reason to oppose) as told above, the second step won't be pressing a simple "Join" button because the user must still choose his/her team(if there's any team in DB).

@andreyv
Copy link
Member

andreyv commented Aug 12, 2021

Yep, I also felt that #1187 (comment) would be more suitable if there was multi-contest mode only. (And by extension the login/register page would be at one path, not multiple.)

FWIW, I don't think single mode can (or should) be removed anytime soon. It is commonly used, and not all CMS parts support multi-contest yet (like ProxyService and RankingWebServer).

There are also these questions:

  • For a single contest residing at /<contest_name>, now there would be an extra navigation step from / — either by manual selection from the trimmed contest list or by redirect. I think this is a regression from the current single-contest mode.
  • What to do with per-participation passwords (and IP login)? Due to this, the user can be logged in in one contest and logged out in another contest at the same time. To accommodate, the single cookie would have to contain O(N) amount of login state, which is effectively the same as the current N cookies implementation.

@wil93
Copy link
Member

wil93 commented Nov 27, 2022

I think this issue was fixed in #1191, feel free to re-open if necessary.

@wil93 wil93 closed this as completed Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants