Can't archive a retrospective when there's no authentication

[mikfreedman]

I am using postfacto on an internal instance of Cloud Foundry with no authentication as policy prevents access to Google SSO.

Creating an archive via admin works fine, however we are hitting a 403 when attempting to archive a retro via the frontend.

Request URL: https://[host]cfapps.io/api/retros/[retro]/archive
Request Method: PUT
Status Code: 403 Forbidden
Remote Address: [IP]
Referrer Policy: strict-origin-when-cross-origin

[mikfreedman]

I want to make a pull request to update #user_allowed_to_perform_admin_action? in api/app/controllers/concerns/retros_auth.rb

from

  def user_allowed_to_perform_admin_action?
    valid_token_provided?
  end

to

  def user_allowed_to_perform_admin_action?
    !GoogleClient.provided? || valid_token_provided?
  end

Where #provided? would check whether auth is setup, but I'm not super familiar with the codebase and wanted advice before getting started

[gshaw-pivotal]

@mikfreedman Taking your approach all the way:

In api/app/controllers/concerns/retros_auth.rb, the user_allowed_to_perform_admin_action function becomes:

def user_allowed_to_perform_admin_action?
    !GOOGLE_CLIENT.auth_setup? || valid_token_provided?
end

Similarly in api/lib/clients/google_client.rb we would add a new method called auth_setup?

def auth_setup?
    ENV.has_key?('GOOGLE_OAUTH_CLIENT_ID')
end

Running locally via docker allows us to archive a retro without having Google Oauth in place.

These changes do break tests, as the test suite has a set of test around ensuring you get 403 forbidden when not authenticated.

The change also would affect the update and update_password endpoints as they use the same authentication checking. If we don't want them affected we would need further modifications were they use an original implementation of the user_allowed_to_perform_admin_action? method and load_and_authenticate_retro_admin.

So it may result in us needing a user_allowed_to_perform_archive_action? which gets the logic proposed above, while the user_allowed_to_perform_admin_action? method is unchanged (or renamed to user_allowed_to_perform_update_action?.

Then we would need two load_and_authenticate_retro_admin method variations eg load_and_authenticate_retro_admin_for_archive and load_and_authenticate_retro_admin_for_update.

Then in api/app/controllers/retros_controller.rb we would modify the before_action block to be like:

before_action :load_and_authenticate_user, only: [:index, :create]
before_action :load_and_authenticate_retro, only: [:show]
before_action :load_and_authenticate_retro_admin_for_archive, only: [:archive]
before_action :load_and_authenticate_retro_admin_for_update, only: [:update, :update_password]

[mikfreedman]

@gshaw-pivotal When there's no auth how do you expect the update and update_password endpoints to work? I feel like update_password doesn't even make sense in a no auth context?

[gshaw-pivotal]

I think the ‘update_password’ is for setting a password on the retro board. Which could be set even if there is no oauth. I would need to check though.

[mikfreedman]

I’m pretty sure you can’t add a retro from the front end unless you are authenticated?

On Thu, Jan 30, 2020 at 11:08 Gavin Shaw notifications@github.com wrote:

I think the ‘update_password’ is for setting a password on the retro board. Which could be set even if there is no oauth. I would need to check though.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pivotal/postfacto/issues/213?email_source=notifications&email_token=AAI7IQDUQLAENWQXQQ7Q3W3RAL3PHA5CNFSM4KHD44GKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKLROQI#issuecomment-580327233, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAI7IQDHNFLIVKT2NDCLUB3RAL3PHANCNFSM4KHD44GA .

[gshaw-pivotal]

Poking at it again and running it in docker etc, I can't work out where on the frontend the update and update_password get called. They are related to a retro or retro board as they are in the retros_controller.rb, but I don't see a corresponding frontend action even in the admin view.

I also had the admin user add a password to a retro board (while running with no gooogle oauth) and the retro board was wide open.

[mikfreedman]

The real question for @liamdarmody and others is.

If auth is not configured, does it make sense for any action (update , update password, archive) to have authorization? I say no - but happy to hear if there's a difference in opinion

[liamdarmody]

This is an interesting bug. Thanks for contributing!

In the absence of authorisation, my immediate thoughts are as follows:

• For retro archive, neither authorisation or retro password should be required.
• For update retro or update password, authorisation should not be required but retro password should be.

What are your thoughts on this @textbook and @crswty?

[seadowg]

@mikfreedman it would be great if you could post a step by step of what's happening for you here. Are you using a public or private retro? Are you trying to archicve via the front end or via the API?

valid_token_provided is checking whether you have authenticated with the retro or not and has nothing to do with Google Auth (which is just used for authenticating for retro creation). When you try and archive a private retro you should have been prompted for the retro password on the way in and so you will be authenticated. For a public retro the front end should prompt you for the retro password when you hit "Archive this retro".

If you're setting Postfacto up for clients and need support for them it would be good to look at trying to allocate some more dedicated resources. As far as I'm aware, there aren't any engineers that are currently paid to work on Postfacto so this is the response times you're looking at.

[mikfreedman]

as per @seadowg's point - no google authenticated retros still need a password for archiving actions.

The challenge is on the client's internal deployment without a password we get no redirect on the 403 like Callum is describing. Just a blank screen!

Per his instructions you need to have a password on a private retro for archiving to work.

The whole flow is kinda weird, we're going to look into it more!

Thanks for your help

[seadowg]

@mikfreedman do you remember how you created the retro? I tried this the other day (creating a retro from the admin panel without a passwrod) when I saw your post but I ended up with a blank ("") password rather than no password and still got the login flow.

[mikfreedman]

@seadowg turns out there was some problem with the internal deployment so that there was no redirect happening to the login page when one attempted to archive a retro and wasn't authenticated.

That seems to have been resolved, so I think we can go ahead and close this issue if everyone is comfortable with the flow being:

When I attempt to archive a retro, and it has no password set, I can enter any text into the password field to continue archiving the retro

Thanks again for your help here!

Paging @kindBeing for a better explanataion

[butzopower]

@liamdarmody How do you feel about the flow being as @mikfreedman suggests.