[openstreetmap/openstreetmap-website] Add oauth scope for redactions (PR #4387)

[Tom Hughes]

@tomhughes commented on this pull request.



> @@ -31,10 +31,10 @@ def initialize(token)
         if user.moderator?
           can [:destroy, :restore], ChangesetComment if scope?(token, :write_api)
           can :destroy, Note if scope?(token, :write_notes)
-          if user&.terms_agreed?
-            can :redact, OldNode if scope?(token, :write_api)
-            can :redact, OldWay if scope?(token, :write_api)
-            can :redact, OldRelation if scope?(token, :write_api)
+          if user&.terms_agreed? && (scope?(token, :write_api) || scope?(token, :write_redactions))
+            can :redact, OldNode
+            can :redact, OldWay
+            can :redact, OldRelation

I'd prefer to leave the guards on the individual `can` methods for consistency - it's the way we do it everywhere else in the file even though it's duplicative.

The double condition is only temporary anyway.

> @@ -0,0 +1,15 @@
+module AuthorizationHelper
+  include ActionView::Helpers::TranslationHelper
+
+  MODERATOR_SCOPES = %w[write_redactions].freeze

This should probably be in `lib/oauth.rb` for consistency?

> @@ -0,0 +1,15 @@
+module AuthorizationHelper
+  include ActionView::Helpers::TranslationHelper
+
+  MODERATOR_SCOPES = %w[write_redactions].freeze
+
+  def authorization_scope(scope)
+    html = []
+    if MODERATOR_SCOPES.include? scope
+      html << image_tag("roles/moderator.png", :srcset => image_path("roles/moderator.svg", :class => "align-text-bottom"), :size => "20x20")
+      html << " "

Putting the marker before the scope name is going to mean the scopes don't lineup properly which doesn't seem right - maybe it should go after the name?

Should we also add the administrator icon to privileged scopes?

> @@ -1,7 +1,7 @@
 module Oauth
   SCOPES = %w[read_prefs write_prefs write_diary write_api read_gpx write_gpx write_notes].freeze
   PRIVILEGED_SCOPES = %w[read_email skip_authorization].freeze
-  OAUTH2_SCOPES = %w[openid].freeze
+  OAUTH2_SCOPES = %w[write_redactions openid].freeze

Should we limit who can create an application that requests moderator permissions to moderators in the way we do for administrators and privileged scopes?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4387#pullrequestreview-1765897459
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/4387/review/1765897459 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20231205/0adb03c1/attachment.htm>