Explicitly capture skipped state.

[ioquatix]

This is just an idea to capture the reason why the controller was authorised, so that it can be later logged and/or disambiguated - i.e. we know the difference between "it was authorized" and "it was skipped"

[Burgestrand]

Thanks! This is neat. I'd like to run this against another set of eyes (@dgmstuart) before merging though.

Thinking out loud here, but at the moment there's no documented public API to retrieve this. Sure, there's an instance variable and I might even argue myself that it's technically public regardless of what's documented. Perhaps a public API to retrieve this information would be good.

What's your own thoughts on this @ioquatix?

[ioquatix]

@Burgestrand We have a rather large application using pundit. We are trying to audit the code base both during CI and with production traffic. That's because CI doesn't have 100% coverage.

We are looking at the following, among other metrics:

• We can check in CI if we are missing authorize calls.
• We can check in production if we are missing authorize calls.

Ideally, we'd like to split production into:

• Didn't call authorize or skip authorize, or
• Called skip authorize at least once, or
• Called authorize at least once.

This is pretty useful information because along with logging the action name, we can determine where we need to spend our time adding authorisation. Bearing in mind this is 10 million+ requests per day.

So to answer your question, right now we are using the instance variable. But I think it would be nice to have a public interface for this. But either is fine for us, we can adapt according to how pundit changes.

I can't think of other scenarios we care about at the moment, but it might change as we develop this further. Certainly this PR would go a long way to us disambiguating the usage (or lack) of authorize.

It would also be slightly helpful for us if we defaulted @_pundit_policy_authorized and @_pundit_policy_scoped to false rather than nil.

Then, in terms of what we care about, it looks something like metric_increment(action_name: self.action_name, authorised: @_pundit_policy_authorized || @_pundit_policy_scoped).

I am happy to provide additional PRs as they align with work requirements.

[Burgestrand]

Thank you @ioquatix!

Both me and @dgmstuart feel it'd be good to have an accessor instead of accessing the ivar directly, but none of us have the bandwidth for it at the moment. If that's something you'd like to work on when time allows that'd be much appreciated.

I don't like to make a new release of Pundit for a change that is technically internal, so this making it into the gem would need to wait until there's something to actually release.

That said, this might make it into the next release even if there's no accessor since there are other PRs to look at that might introduce changes (features/bugfixes) that are worth in releasing on their own.

[ioquatix]

I will work on an updated interface next week.