Memo engagement + LinkedIn sign-in (via omniauth-linkedin-openid) - #74
Merged
Conversation
…n tickets Reworks the memo engagement feature so endorsements/critiques belong to a real authenticated User (resolved from a Doorkeeper access token), replacing the anonymous 15-min LinkedIn 'verification ticket' flow. - Sign in with LinkedIn (browser OIDC -> Devise session) via Users::LinkedinController, reusing the LinkedinOidc service; feeds the Doorkeeper authorize flow. - engagements re-keyed to user_id (drops inline identity columns); unique [memo, type, user]. Identity/postal read through the User. - Endorse/critique endpoints require a Doorkeeper token and 422 when the user has no postal code; /me exposes postal_code + engagement_ready and PATCH /me sets it. - User: from_linkedin upsert; postal_code no longer required at account creation (collected at signup/first engagement), valid Canadian format enforced. - Removes VerificationTicket, the api/v1/auth/linkedin popup controller/view, and the LinkedinVerified concern. - Tests: user, engagement model, endorse/critique API, /me, LinkedIn sign-in.
The bespoke LinkedinOidc service + Users::LinkedinController hand-rolled the OAuth code exchange, JWKS id_token verification, and CSRF/redirect dance. Live testing surfaced two bugs baked into that approach: the id_token issuer is https://www.linkedin.com/oauth (the code expected https://www.linkedin.com), and LinkedIn never returns the nonce claim, so verification always failed. Switch to omniauth-linkedin-openid — the maintained strategy, on the same OmniAuth + Devise path already used for Google. It pins LinkedIn's OIDC endpoints and absorbs the issuer/nonce quirks, so both bugs disappear by construction and ~165 lines of bespoke code go away. - Add omniauth-linkedin-openid; register :linkedin in devise.rb - User.from_linkedin maps the OmniAuth auth hash (name from extra.raw_info, avatar from info.picture_url) - New Users::OmniauthCallbacksController preserves the return_to and admin/profile redirect logic for the Doorkeeper flow - Sign-in button POSTs to the omniauth authorize path (CSRF protection) - Delete LinkedinOidc and Users::LinkedinController - Rewrite tests to use OmniAuth test mode
…nt-users # Conflicts: # db/seeds/trade_barriers_jurisdictions.rb
- Delete db/structure-dev.sql — orphan pg_dump copy; Rails only reads db/structure.sql and nothing references it (would silently drift). - Delete Dockerfile.dev — no docker-compose file exists to consume it. - Drop the RAILS_DEVELOPMENT_HOSTS hosts hook (part of the same absent docker-compose setup). - Guard the dev-only memo_engagements sample seed behind Rails.env.development? so db:seed can't create a fake memo + fake users in production. - Drop the redundant standalone index_engagements_on_memo_id — both composite indexes already lead with memo_id, so it only added write overhead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backend for reader memo engagement (endorse / critique) plus "Sign In with LinkedIn" so TradingPost readers can self-register and engage. Pairs with the TradingPost PR BuildCanada/TradingPost#37.
The engagement pipeline (Engagement/Endorsement/Critique models,
/api/v1/memos/:slug/endorsements|critiques,/api/v1/me, seeds, local Docker rig) is the work from #38, rebased onto currentmain.On top of that, this branch replaces the hand-rolled LinkedIn OIDC with the maintained
omniauth-linkedin-openidstrategy.Why the auth rewrite
The original
LinkedinOidcservice +Users::LinkedinControllerhand-rolled the code exchange, JWKS id_token verification, and CSRF/redirect dance. Live end-to-end testing against real LinkedIn surfaced two bugs baked into that approach:https://www.linkedin.com/oauth, nothttps://www.linkedin.com. Verification always failed.nonceclaim in the id_token, sopayload["nonce"] == noncewas alwaysnil == "…".Rather than patch the bespoke code, this switches to
omniauth-linkedin-openid(~360k downloads), on the same OmniAuth + Devise path already used for Google. It pins LinkedIn's OIDC endpoints and absorbs both quirks, so the bugs disappear by construction and ~165 lines of hand-rolled code are deleted.Changes (auth commit)
omniauth-linkedin-openid; register:linkedinindevise.rbUser.from_linkedinmaps the OmniAuth auth hash (name fromextra.raw_info, avatar frominfo.picture_url)Users::OmniauthCallbacksControllerpreserves thereturn_to+ admin/profile redirect logic the Doorkeeper flow depends onLinkedinOidcandUsers::LinkedinControllerTesting
/oauth/authorizeissues a code to TradingPost → token exchange →/api/v1/me→ endorse a memo writes a realEndorsementrowNote
This branch supersedes #38 — it contains #38's commits (rebased) plus the auth rewrite. Suggest closing #38 in favour of this once reviewed.