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

Don't show ips to non-administrators, Fixes #694 #739

Merged
merged 5 commits into from
Apr 17, 2017
Merged

Don't show ips to non-administrators, Fixes #694 #739

merged 5 commits into from
Apr 17, 2017

Conversation

edg-l
Copy link
Member

@edg-l edg-l commented Apr 14, 2017

People with non-administrator rights won't be able to see ips (mod, helper).

@edg-l edg-l changed the title Don't show ips to non-administrators, fixes #694 Don't show ips to non-administrators, Fixes https://github.com/ddnet/ddnet/issues/694 Apr 14, 2017
@edg-l edg-l changed the title Don't show ips to non-administrators, Fixes https://github.com/ddnet/ddnet/issues/694 Don't show ips to non-administrators, Fixes #694 Apr 14, 2017
@Learath2
Copy link
Member

There is probably something related to mute you are missing. Otherwise looks good.

@heinrich5991
Copy link
Member

There's also a bit missing wrt. players joining and leaving. If you want to solve it this way, you could search for net_addr_str (and GetClientAddr) in the code base and review each use.

@edg-l
Copy link
Member Author

edg-l commented Apr 14, 2017

Now it should cover all cases, also i renamed the var c to a more descriptive name "ClientID" :)

@Learath2
Copy link
Member

Learath2 commented Apr 15, 2017

Well I might've used a wrapper for net_addr_str which turns it into xxx.xxx.xxx.xxx but this works too. It looks better this way I guess.

@heinrich5991
Copy link
Member

The newly added cases (player joining, etc.) only show the IP addresses of admins to everyone, instead of only showing the IP addresses of everyone to admins.

Copy link
Member

@heinrich5991 heinrich5991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix that the blacklist output is only shown if the affected person is an administrator. Otherwise, this looks good to me.

char aBuf[256];
str_format(aBuf, sizeof(aBuf), "client dropped. cid=%d addr=%s reason='%s'", ClientID, aAddrStr, pReason);

char aBuf[256];str_format(aBuf, sizeof(aBuf), "client dropped. cid=%d addr=%s reason='%s'", ClientID, aAddrStr, pReason);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A newline was accidently deleted here.

str_format(aBuf, sizeof(aBuf), "ClientID=%d addr=%s secure=%s blacklisted", ClientID, aAddrStr, m_NetServer.HasSecurityToken(ClientID)?"yes":"no");
else
str_format(aBuf, sizeof(aBuf), "ClientID=%d secure=%s blacklisted", ClientID, m_NetServer.HasSecurityToken(ClientID) ? "yes" : "no");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still only shows the IP addresses of admins, and of no one else, if I read the code correctly.

@edg-l
Copy link
Member Author

edg-l commented Apr 17, 2017

I finally got it, wathever string you print to the level IConsole::OUTPUT_LEVEL_STANDARD will be shown to all staff levels,

But if you print to the IConsole::OUTPUT_LEVEL_ADDINFO only rcon admins and server console will see it

So now should be fine.

@heinrich5991
Copy link
Member

I finally got it, wathever string you print to the level IConsole::OUTPUT_LEVEL_STANDARD will be shown to all staff levels,

What I read from the code was that no one sees console output unless you're admin or someone has just executed a command on your level.

@heinrich5991 heinrich5991 merged commit 3a47385 into ddnet:master Apr 17, 2017
@heinrich5991
Copy link
Member

Thanks!

@edg-l edg-l deleted the pr_hide_ip_nonadmin branch April 17, 2017 10:16
black-roland pushed a commit to black-roland/goreddnet that referenced this pull request Jun 4, 2017
People with non-administrator rights won't be able to see ips (mod, helper).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants