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

Fix oper override message when viewing a list mode #1418

Open
wants to merge 1 commit into
base: insp3
Choose a base branch
from

Conversation

B00mX0r
Copy link
Contributor

@B00mX0r B00mX0r commented Nov 26, 2017

Currently, oper override executes a MODE override attempt every time a MODE command with a higher adding level than the user has is run. Consequently, this results in an oper override announcement showing when an oper simply did /mode #chan +b to view a list mode rather than modify it.

Because the m_hidelist module explicitly provides for a channels/auspex permission to bypass its restrictions, it is outside of oper override's jurisdiction to touch or care about list modes. This PR makes oper override completely ignore viewing list modes, meaning that a MODE override permission will not replace m_hidelist's channels/auspex requirement for a bypass, and that an oper innocently viewing a listmode will not trigger the override announcement.

This fixes #425.

@B00mX0r B00mX0r force-pushed the master+fix_listoverride branch 2 times, most recently from 081adc2 to a0cb004 Compare December 12, 2017 06:36
@B00mX0r B00mX0r force-pushed the master+fix_listoverride branch 2 times, most recently from c8046a7 to 9a2f34a Compare December 22, 2017 08:37
@SadieCat
Copy link
Member

SadieCat commented Apr 6, 2018

Hey,

Sorry for taking so long to review this. It seems like I managed to miss it.

Oper override would execute for MODE every time a mode in the command had an "adding" permission higher than the user's current permission. This would result in an oper override announcement showing when an oper simply did /mode #chan +b.

Sending an override announcement is valid behaviour when overriding <security:hidemodes>. The issue is that sometimes when an oper views a channel's list modes it does not take into account whether the oper's access in that channel is high enough to allow them to list modes without overriding. In such a case no message should be sent.

Because hideopers gives channels/auspex permission to bypass it, this patch does not use oper override to override the viewing of list modes with hideopers in place.

The hideopers module does not use channels/auspex. It does use users/auspex but only in the context of checking whether an oper can override hideoper which is not relevant to the override module.

@SadieCat SadieCat added the waiting for author Waiting for the author to respond label Apr 6, 2018
@B00mX0r
Copy link
Contributor Author

B00mX0r commented Apr 8, 2018

@SaberUK My sincere apologies. The PR description was wrong, as I accidentally referred to the incorrect module. I have rewritten the description to properly reflect the changes that this PR implements.

@SadieCat SadieCat removed the waiting for author Waiting for the author to respond label Apr 8, 2018
@B00mX0r B00mX0r force-pushed the master+fix_listoverride branch 2 times, most recently from cb386fa to 868d6a7 Compare April 9, 2018 00:18

#include "event.h"

class HidelistEventListener : public Events::ModuleEventListener
Copy link
Member

Choose a reason for hiding this comment

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

HideListEventListwnwe.

{
}

/** Called whenever a /WHOIS is performed by a local user.
Copy link
Member

Choose a reason for hiding this comment

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

Update the code documentation too.

/** Called whenever a /WHOIS is performed by a local user.
* @param whois Whois context, can be used to send numerics
*/
virtual ModResult OnListDenied(User* user, Channel* chan, const std::string& modename) = 0;
Copy link
Member

@SadieCat SadieCat Apr 9, 2018

Choose a reason for hiding this comment

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

Pass a list mode handler here instead of just the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have this information in the ModeWatcher class. Also, the mode name just needs to be passed for the m_override snomask announcement.

@B00mX0r
Copy link
Contributor Author

B00mX0r commented Apr 11, 2018

I renamed OnListDenied to OnListDeny to better reflect that the denial is dependent on the MOD_RESULT from this call.

@B00mX0r B00mX0r changed the base branch from master to insp3 January 31, 2019 09:32
@B00mX0r B00mX0r force-pushed the master+fix_listoverride branch 2 times, most recently from 942a758 to ec7179d Compare January 31, 2019 09:45
@B00mX0r
Copy link
Contributor Author

B00mX0r commented Jan 31, 2019

Targeting v3 instead of v2 because it's in RC stage now

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

Successfully merging this pull request may close these issues.

[2.0] m_override reports non-overrides
2 participants