Closed
Bug 884681
Opened 12 years ago
Closed 12 years ago
Username persists in session after logout
Categories
(Webmaker Graveyard :: webmaker.org, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: kate, Assigned: mjschranz)
References
Details
Attachments
(2 files, 3 obsolete files)
After the user initiates a logout, req.session.email is null but req.session.username is still set to the user's username
| Assignee | ||
Updated•12 years ago
|
Assignee: nobody → schranz.m
| Assignee | ||
Comment 1•12 years ago
|
||
https://github.com/mozilla/login.webmaker.org/pull/112
https://github.com/mozilla/node-webmaker-loginapi/pull/17
Double Patch!
Attachment #764719 -
Flags: review?(pomax)
Attachment #764719 -
Flags: review?(jon)
Attachment #764719 -
Flags: review?(david.humphrey)
| Assignee | ||
Comment 2•12 years ago
|
||
Comment on attachment 764719 [details]
TWO PATCHES IN ONE
Jon brought up doing this with express-persona.
Going to cancel these reviews.
Attachment #764719 -
Flags: review?(pomax)
Attachment #764719 -
Flags: review?(jon)
Attachment #764719 -
Flags: review?(david.humphrey)
Updated•12 years ago
|
Attachment #764719 -
Attachment is obsolete: true
Comment 4•12 years ago
|
||
Updated•12 years ago
|
Status: NEW → ASSIGNED
| Assignee | ||
Comment 5•12 years ago
|
||
Login Patch
Attachment #764779 -
Attachment is obsolete: true
Attachment #764805 -
Flags: review?(pomax)
Attachment #764805 -
Flags: review?(jon)
Attachment #764805 -
Flags: review?(david.humphrey)
| Assignee | ||
Comment 6•12 years ago
|
||
Node Module Patch.
Attachment #764806 -
Flags: review?(pomax)
Attachment #764806 -
Flags: review?(jon)
Attachment #764806 -
Flags: review?(david.humphrey)
| Assignee | ||
Comment 7•12 years ago
|
||
Use https://github.com/mjschranz/popcorn.webmaker.org/tree/testPatch884681 as a test branch in Popcorn Maker.
Comment on attachment 764805 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/114
this seems pretty sane to me. We're starting to nest callbacks pretty deep now, though.
Attachment #764805 -
Flags: review?(pomax) → review+
Comment 10•12 years ago
|
||
Comment on attachment 764806 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/18
looks sane, with comments in the pull request
Attachment #764806 -
Flags: review?(pomax) → review+
| Assignee | ||
Comment 11•12 years ago
|
||
Comment on attachment 764805 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/114
Rebased up the login patch. Did review fixes for the node module. Would still like more eyes on this.
Attachment #764805 -
Flags: review+ → review?(pomax)
Comment 12•12 years ago
|
||
Comment on attachment 764806 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/18
This is an improvement, but we can go even further. Inside node-webmaker-loginapi:
module.exports = function(app, options) {
var persona = require("express-persona");
options.verifyResponse = function(err, req, res, email) {
// your current personaLogin code
};
options.logoutResponse = function(err, req, res) {
// your current personaLogout code
};
persona(app, options);
return;
};
This way we could remove the need to require("express-persona") within our apps, and just use this module.
Attachment #764806 -
Flags: review?(jon)
Attachment #764806 -
Flags: review?(david.humphrey)
Attachment #764806 -
Flags: review-
Comment 13•12 years ago
|
||
Comment on attachment 764805 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/114
r- because I suspect you'll be able to remove all the GET /user/:email code when you change the node module patch.
Attachment #764805 -
Flags: review?(pomax)
Attachment #764805 -
Flags: review?(jon)
Attachment #764805 -
Flags: review?(david.humphrey)
Attachment #764805 -
Flags: review-
| Assignee | ||
Comment 14•12 years ago
|
||
Comment on attachment 764805 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/114
Review fixes.
Attachment #764805 -
Flags: review- → review?(jon)
| Assignee | ||
Comment 15•12 years ago
|
||
Comment on attachment 764806 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/18
Review Fixes.
Attachment #764806 -
Flags: review- → review?(jon)
Comment 16•12 years ago
|
||
Comment on attachment 764806 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/18
comments in PR
Attachment #764806 -
Flags: review?(jon) → review-
Comment 17•12 years ago
|
||
Comment on attachment 764805 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/114
Getting a really weird travis fail here...
Attachment #764805 -
Flags: review?(jon) → review-
| Assignee | ||
Comment 18•12 years ago
|
||
Comment on attachment 764805 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/114
I don't see any fails on Travis, at least not after a quick update for work done on the module.
Attachment #764805 -
Flags: review- → review?(jon)
| Assignee | ||
Comment 19•12 years ago
|
||
Comment on attachment 764806 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/18
Fixed review points.
Attachment #764806 -
Flags: review- → review?(jon)
| Assignee | ||
Updated•12 years ago
|
Attachment #764806 -
Flags: review+ → review?(pomax)
| Assignee | ||
Comment 21•12 years ago
|
||
Comment on attachment 764805 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/114
Had to make updates, so this will need to be updated.
Attachment #764805 -
Flags: review?(jon) → review?(pomax)
| Assignee | ||
Comment 22•12 years ago
|
||
Here's it working with webmaker.org.
Obviously when the actual node module patch lands I'll have an appropriate version there in the package.json
Attachment #770916 -
Flags: review?(pomax)
Comment 23•12 years ago
|
||
This works quite well! I'd like to r+, but there's a few attachments that probably don't need an r+ since they were testing-only bits. If you clear those, I'll r+ the remainder, and then we can see how many components need to update at the same time for pushing this to staging.
| Assignee | ||
Updated•12 years ago
|
Attachment #770916 -
Attachment is obsolete: true
Attachment #770916 -
Flags: review?(pomax)
Comment 24•12 years ago
|
||
Commit pushed to master at https://github.com/mozilla/login.webmaker.org
https://github.com/mozilla/login.webmaker.org/commit/8d7ce8d58e55458a6a831b591bb8d4f893d71357
Bug 884681 - Adapt SSO-UX to account for express-persona adding username to session
| Assignee | ||
Comment 25•12 years ago
|
||
Both of the above patches have been landed. The follow up bugs are listed as bugs this blocks.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Attachment #764806 -
Flags: review?(pomax)
Attachment #764806 -
Flags: review?(jon)
| Assignee | ||
Updated•12 years ago
|
Attachment #764805 -
Flags: review?(pomax)
Updated•12 years ago
|
Attachment mime type: text/plain text/plain text/plain → text/x-github-pull-request text/x-github-pull-request text/x-github-pull-request
You need to log in
before you can comment on or make changes to this bug.
Description
•