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

Publicly Listing of all test attempts with IP included #4183

Closed
sasnycon opened this issue Feb 22, 2022 · 13 comments · Fixed by #4189
Closed

Publicly Listing of all test attempts with IP included #4183

sasnycon opened this issue Feb 22, 2022 · 13 comments · Fixed by #4189

Comments

@sasnycon
Copy link

sasnycon commented Feb 22, 2022

Describe
All test attempts of non logged users are listed publicly with included IP address. There is not way to disable this or at least I can't find one.

To Reproduce
Steps to reproduce the behavior:

  1. Create a course and make it Public
  2. Add a test and a question
  3. Logout
  4. Attempt to answer the test questions.
  5. when you finish, go back on the "start Test" step and you will see your IP and your attempt.
  6. This is now visible for the entire world that's not logged in.

Expected behavior
You should not see the IP of the other people that are attempting the test.

Screenshots
image

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Chrome and Firefox.

**Server

  • OS: CentOS 7
  • Version of Chamilo: 1.11.16
  • PHP Version: 7.3

Additional context
Add any other context about the problem here.

@fabiowoj
Copy link

I want to know too the correct settings to change this. But, you can do the bad way.
Put IP cell in blank at file main/exercise/overview.php

@sasnycon
Copy link
Author

I want to know too the correct settings to change this. But, you can do the bad way. Put IP cell in blank at file main/exercise/overview.php

I have only found a way to disable the cell header in this file, between line 382 and 434.
image

Where exactly is the statement for the cell containing the IP itself?

@ywarnier
Copy link
Member

@sasnycon here: https://github.com/chamilo/chamilo-lms/blob/1.11.x/main/exercise/overview.php#L313

Could you explain why this is an issue in your case ? Would removing this column automatically in all courses marked "public" be the right solution ? Or should we rather check whether the user taking the test is an anonymous (non identified) user ? (trying to find what is the best solution with a practical case)

@ywarnier
Copy link
Member

I mean I need a little more info, because maybe it is an issue to store this IP in the database in the first place, but I'm not sure what your use case is... Is it related to GDPR or something else ?

@sasnycon
Copy link
Author

sasnycon commented Feb 23, 2022

Hi, ywarnier,

Thank you for your reply.

I will try to give as much information as I can. Please find my answers below:

Q: Could you explain why this is an issue in your case ?
A: I do not think that it's secure to show a public history of IP addresses and times of every quiz attempt. I feel naked seeing this logs publicly. And to be honest there is not logic behind that behavior. Every other attempt history is private and personal for the users that are logged in, but every attempt of any non-logged in visitor is public for all other non-logged in visitors.

Q: Would removing this column automatically in all courses marked "public" be the right solution ? Or should we rather check whether the user taking the test is an anonymous (non identified) user ? (trying to find what is the best solution with a practical case)
A: I personally will not show any logs like those publicly to all non-loghged in users (to any user actually, that's not the one that made the attempt). You can tie them to a cookie or something to show only the attempts logs of the specific user, but not like that (all logs, of all attempts of non-logged in users, visible to all non-logged in visitors). Best case scenario not show any attempt history of and to visitors. Those should be private or only accessed by admins and teachers owning the specific course.

Q: Is it related to GDPR or something else ?
A: As for the GDPR I know that every information that can identify a visitor is considered a private information and should not be public. I'm not GDPR expert but I know that in some cases the IP address is considered private information, as it can identify the user and you need to collect them only if this is required for the operation of the system and security. In any case, showing publicly logs including IP addresses like that is not ok.

@ywarnier
Copy link
Member

Thank you for the extra info. It helps understand how to better fix this.

Just so you know, there is a quick-fix mechanism to show this history based on cookies: you have to create more anonymous users (it's a bit tricky as you have to create normal users, than change their "status" to 6 in the database, but it's not that complex). If there are more than one anonymous users, then Chamilo will naturally "attach" each new anonymous user to an anonymous account and only show them their own history. This has a limit, though: as soon as you don't have enough anonymous users in the database to match new users connecting without logging in, it will start picking in the list of previous anonymous users and attach the new user to one of these (and show the corresponding history).

Anyway, we are left with the task to automatically hide this IP address when ay anonymous user looks at the history of previous attempts.
We do want to keep this list, though, because it helps the user understand that, by not connecting, (s)he is sharing history with other unknown users.

@christianbeeznest
Copy link
Contributor

Hi @ywarnier , the request is done in this PR #4189

Thanks for confirmation.

@Coursenligne
Copy link

Hy Guys,
Sorry to interfer.
Maybe not visible to all, but as an Admin,it is important to be able to track IPs. As you already know, this is a good way to track hacking attempts.
Maybe this could be a tick box somewhere in administration of the portal to disable displaying for all, but for Admin it's really usefull.

@lonesomewalker
Copy link

You can track this elsewhere (fail2ban i.e.).
By GDPR you cannot simply collect somehow private data.
Yes, from the "bad guy" perspective it is okay, but it says "privacy by design, privacy by default".

But nevermind...

@sasnycon
Copy link
Author

Hy Guys, Sorry to interfer. Maybe not visible to all, but as an Admin,it is important to be able to track IPs. As you already know, this is a good way to track hacking attempts. Maybe this could be a tick box somewhere in administration of the portal to disable displaying for all, but for Admin it's really usefull.

To collect data and to make it publicly visible are 2 completely different things.
I'm ok to collect that data and to make it visible for teachers and the specific user that owns the attempts, but against making it visible to everyone.

ywarnier added a commit that referenced this issue Apr 25, 2022
Exercise - Hide column IP when user is anonymous in overview page #4183
@ywarnier
Copy link
Member

@sasnycon Not sure how familiar your are with testing code changes sent through Git, but we'd love your feedback on the changes mentioned above, that should hide the IPs when users are anonymous, and show them only to the corresponding users.

Otherwise we'll just consider it fixed as our internal testing suggests.

@sasnycon
Copy link
Author

Hy Guys, Sorry to interfer. Maybe not visible to all, but as an Admin,it is important to be able to track IPs. As you already know, this is a good way to track hacking attempts. Maybe this could be a tick box somewhere in administration of the portal to disable displaying for all, but for Admin it's really usefull.

This has nothing to do with the logs. It;s regarding sharing IPs to everyone in public. It's an information that admins should not share anyway by any security standards.

@sasnycon
Copy link
Author

@sasnycon Not sure how familiar your are with testing code changes sent through Git, but we'd love your feedback on the changes mentioned above, that should hide the IPs when users are anonymous, and show them only to the corresponding users.

Otherwise we'll just consider it fixed as our internal testing suggests.

I can't test it as I ave already changed the code manually as per the initial suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants