Skip to content

Fixed make deploy* targets & added key param for securing user cookies#100

Closed
mruac wants to merge 6 commits intoDeer-Spangle:masterfrom
mruac:master
Closed

Fixed make deploy* targets & added key param for securing user cookies#100
mruac wants to merge 6 commits intoDeer-Spangle:masterfrom
mruac:master

Conversation

@mruac
Copy link
Copy Markdown
Contributor

@mruac mruac commented Nov 5, 2022

Added double quotes around env vars in Makefile so that passed vars are included properly.

Added key support in URL query parameter or settings.yml for securing endpoints where login cookie is required.

mruac added 4 commits November 5, 2022 14:35
The `ensure_login!` method was always returning the `FALoginCookieError` due to the `@user_cookie` returning true, so I just made it false so it won't return the error.
Fix false FALoginCookieError
fixed how vars were being passed to make deploy* commands
added key query param for when accessing pages that require login cookie
@mruac mruac closed this Nov 5, 2022
@mruac
Copy link
Copy Markdown
Contributor Author

mruac commented Nov 6, 2022

Turns out I had messed up my docker build. The code was working in the first place. Reopening this PR!
The idea behind this is that if you are going to exposing endpoints that require login cookies, then you may as well secure them to prevent unintended users favving / creating journals on your behalf.

@mruac mruac reopened this Nov 6, 2022
@Deer-Spangle
Copy link
Copy Markdown
Owner

Sorry for taking so long to get to these, I had a very busy end of year, just starting to get back into the swing of things with my coding projects.

I'm not sure the key parameter is needed, as I said on your other PR (#99), the user provides their own FA login cookies via the HTTP_FA_COOKIE header, which is read in the before method. So they can only post journals and fav art if they provide that header, which means they can't post journals and fav art on your behalf.

Double quoting the variables there though is quite wise! That should definitely be done. I'm not sure whether I can merge just that bit, or whether I should just replicate it myself

@Deer-Spangle
Copy link
Copy Markdown
Owner

Oh darnit, I was trying out the github.dev editor, hoping I could add a commit to this PR and then land the Makefile stuff, and it pushed to your fork's master branch, sorry!

…PR, and not to your forked repo)"

This reverts commit abaa8ca.
@Deer-Spangle
Copy link
Copy Markdown
Owner

Okay, cherrypicked your Makefile change in here a34d249 Thank you!

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.

2 participants