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

MeB OAuth - latest iteration #477

Open
wants to merge 93 commits into
base: master
Choose a base branch
from
Open

MeB OAuth - latest iteration #477

wants to merge 93 commits into from

Conversation

amCap1712
Copy link
Member

No description provided.

amCap1712 and others added 16 commits August 30, 2024 22:08
security hardening by detecting code reuse, adding arbitrary fragment. improved
token format. made static resources work in dev and prod.
add post method to authorization endpoint
need to look into security concerns about previous oauth attempts' data left
in session. because the post data needs to be saved in session while the user is
redirected to login. considered server side sessions but not sure if its enough.
will probably need timestamps and some hashing to invalidate and careful checks
to ensure data between two requests is not mixed. not worth the pains for now.
Copy link

github-actions bot commented Aug 30, 2024

Test Results

179 tests  +112   109 ✅ +42   26s ⏱️ +17s
  1 suites ±  0     5 💤 + 5 
  1 files   ±  0    65 ❌ +65 

For more details on these failures, see this check.

Results for commit 7271352. ± Comparison against base commit d35a224.

♻️ This comment has been updated with latest results.

@MonkeyDo
Copy link
Member

I have applied the last bits of feedback to the OAuth applications pages based on the comments of the design figma
https://www.figma.com/design/L0qNv3z5vSkNDoiX7BzaY5/MetaBrainz-redesign?node-id=0-1&node-type=canvas

We might eventually want to restyle these pages, but as aerozol commented on that figma: "Looks good, just needs to be functional"

Comment on lines +46 to +57
<div className="form-group">
<div className="col-md-offset-3 col-md-1">
<a href={cancel_url} className="btn btn-default">
Cancel
</a>
</div>
<div className="col-md-1" style={{ marginLeft: "8px" }}>
<button type="submit" className="btn btn-danger">
Delete
</button>
</div>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

On smaller sizes the button stack up weirdly.
image

You can use a bootstrap btn-group instead of using col-* utilities like so:

Suggested change
<div className="form-group">
<div className="col-md-offset-3 col-md-1">
<a href={cancel_url} className="btn btn-default">
Cancel
</a>
</div>
<div className="col-md-1" style={{ marginLeft: "8px" }}>
<button type="submit" className="btn btn-danger">
Delete
</button>
</div>
</div>
<div className="btn-group">
<a href={cancel_url} className="btn btn-default">
Cancel
</a>
<button type="submit" className="btn btn-danger" style={{ marginLeft: "8px" }}>
Delete
</button>
</div>

Result
image

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