Persona enabled node version

[ozten]

Wanted to get a pull request going for feedback.

This adds Persona to the footer, so you can log in with an email address.

[ozten]

@nm - I'd love any help and feedback. I'll get this patch cleaned up, based on the TODOs.

[ozten]

Forced pushed an update:

• Finished Persona based middleware
• Page uses session state to properly display signin/signout UI

[nrn]

Maybe switch to mikeal/request to make this section easier to follow?

[nrn]

Nice, I like how this is shaping up!

[paul90]

should not be hard coded - there is already a parameter for this, use "#{argv.u}"

[ozten]

Hi nm. I think we're in pretty good shape. I've done all the TODOs I had written for myself.

What do you think? To try it out.... Remove your data/status/open_id.identity file, so your wiki is unclaimed.

[paul90]

Just had a quick look, on Windows 7, the server crashes when I try to login (with either FF21, or Chrome).

F:\GitHub\Smallest-Federated-Wiki\server\express\lib\persona_auth.coffee:58
                  setOwner(verified.email, function() {
                  ^
ReferenceError: setOwner is not defined
    at IncomingMessage.<anonymous> (F:\GitHub\Smallest-Federated-Wiki\server\express\lib\persona_auth.coffee:58:19)
    at IncomingMessage.EventEmitter.emit (events.js:117:20)
    at _stream_readable.js:883:14
    at process._tickDomainCallback (node.js:459:13)

also notice that if I use IE 10 that the login button does not get displayed.

and, if I try editting an unclaimed site - which should work - I am getting browser display corrupted with Error on /page/welcome-visitors/action: Forbidden, and rejecting /page/welcome-visitors/action in the log. This looks a bit like a repeat of the problem we had earlier with the auth state being indeterminate. Should have some time to look at that later.

[paul90]

This looks a bit like a repeat of the problem we had earlier with the auth state being indeterminate.

While it looks, in server.coffee, as if like the app.get ///([a-z0-9-]+)\.html$///, (req, res, next) -> section needs owner: owner adding to the info data structure, and I guess console.log 'req.session', req.session as well; that is not the solution to the problem.

[ozten]

Sorry about that. I'll try a fresh deployment, to try to reproduce the crash.

While it looks, in server.coffee, as if like the app.get ///([a-z0-9-]+).html$///, (req, res, next) -> section needs owner: owner adding to the info data structure, and I guess console.log 'req.session', req.session as well; that is not the solution to the problem.

I don't think owner should be in that line of code.

Maybe my coffee-script is wrong, here is what I'm trying to do:
In server.coffee where owner is in scope... create a function which you can call and it returns that owner value.

At runtime for each request... pass this function as an argument to authenticate_session and to verify_assertion

The persona_auth module exposes two functions. Each function, called when processing a request, passes this getOwner function, so that the code can find out what owner is set too in server.coffeee.

Every request would hit the authentication middleware which will call getOwner() which is passed in.

[paul90]

No, that's not it. Forgot to say that regenerating client.js was one of the things I tried. So, still getting

[ 'welcome-visitors' ]
[ 'isAuthenticated? owner=',
  '',
  { cookie:
     { path: '/',
       _expires: null,
       originalMaxAge: null,
       httpOnly: true } },
  undefined ]
req.session { cookie:
   { path: '/',
     _expires: null,
     originalMaxAge: null,
     httpOnly: true } }
[ 'isAuthenticated? owner=',
  '',
  { cookie:
     { path: '/',
       _expires: null,
       originalMaxAge: null,
       httpOnly: true } },
  undefined ]

F:\GitHub\Smallest-Federated-Wiki\server\express\lib\persona_auth.coffee:58
                  setOwner(verified.email, function() {
                  ^
ReferenceError: setOwner is not defined
    at IncomingMessage.<anonymous> (F:\GitHub\Smallest-Federated-Wiki\server\express\lib\persona_auth.coffee:58:19)
    at IncomingMessage.EventEmitter.emit (events.js:117:20)
    at _stream_readable.js:883:14
    at process._tickDomainCallback (node.js:459:13)

I also notice that the Mocha tests no longer appear to work - the browser window stays empty.

The problem with not being able to edit an unclaimed site is still there.

However, if I forget to clear down ownership, by removing open_id.identity: -

• if I am not logged into persona there is no problem with making changes and them getting saved into the browsers local store.
• if try and login, I get the OOPS - this is not your wiki. The same as currently with OpenID.
• if I am already logged in (from the failed login and claim above) I also get the OOPS error - this would cause problems in a wiki farm, preventing you from moving between wikis you own and those owned by others while logged in. Also raises a question about what happens when you go to an unclaimed wiki in the farm - probably need a separate claim button.

[ozten]

Finished deploying on dotCloud. I didn't reproduce any of these problems, I'll keep digging.

You can see it here:
http://aokwiki-ozten.dotcloud.com/view/welcome-visitors

[paul90]

The site is already claimed, by you, so the main cases that I am reporting problems with can't be recreated there...

Given that the last commit, be97430, just contained updated client.js and testclient.js - and the problems appear to be server side - is there still some of the server code not pushed to github???

Oh, and if you are starting with a clean install, so an empty data directory, the server code now blocks the creating of favicon.png - but it also fails to clone welcome-vistors from the default data - this all points to problems in the server code. This is just with viewing an new, unclaimed, site - not got to login, and claiming, yet...

[WardCunningham]

I hope to see @ozten at the Indie Web Camp this weekend. Maybe we can work through some of these install cases then. Here is the sort of messaging I'd expect to see in the footer using parens to indicate buttons:

(login) as ward
(logout) as ward
(claim this site with your email)

Presumably the login case would know my full email, ward@c2.com, and enter the persona verification with the full email address from storage. We currently confuse visitors by suggesting that they should login to sites that they cannot own. After we complete the login process (with openid or persona) we report "this is not your wiki". Gack.

I'll have to think through with Austin how this could work when I want to use multiple email addresses interchangeably. Also, another case to consider is when, say, any c2.com login will grant read access but only ward@c2.com grants write access.

[paul90]

Not sure what happened to the post I added late last night, some thoughts about some of the problem edge cases, but...

I think it may be heading towards a far larger discussion about the security model. It is probably best to take elsewhere so we don't overload this thread with the bigger picture.

Oh, and I forgot to say, it is good to see a Persona implementation here - it should have a permanence that some OpenID providers don't have, but that is another problem...

[ozten]

@paul90 +1 thanks for keeping this pull request focused and small.

[ozten]

This fixed one of the failing unit tests.

[paul90]

Not too sure what has happened, as long as the site has already been claimed there doesn't appear to be a problem - logout now works correctly.

However, if the site is not claimed, the page load falls into a loop - have not yet managed to workout what is causing this. The following is grabbed from firebug, showing the network calls.

Will probably get a chance to look at this later.

[paul90]

This looks a bit like Problem with Persona and Safari Web-browser on Windows (users unexpectedly and incorrectly logged out!), though of course here the users is not yet logged in!

Looking at the Persona issues, there seems to a number of other issues that may be related - but it looks as if putting the page refresh is being done in the wrong place.

While looking at this I notice that this is not working with IE10 - the sign in button does not get displayed, unless I have the developer tools running! Most strange.

More later....

[ozten]

I didn't test a clean install. these are bugs in my coffee script code, not in Persona proper.

On a clean install, the page refreshes because
var owner = '';

That should either be null or a valid email address. I'll isolate the bug and fix. Sorry about that.

[ozten]

This could be written better, but I don't really know Coffee, so this is the best I could do.

When I had this as a one liner, it was the source of the refresh bug. Maybe land the patch with this and clean up in a refactor?

[nrn]

How about req.isAuthenticated() and req.session.email ?

[paul90]

Still appears to be looping with a clean install :(

Adding an alert to onlogout shows it is getting called, and causing the loop. Still can't see where this is getting called from... Adding a test to see if the owner is not null, while stopping the loop also prevents the logout from working....

[paul90]

Getting more confused - if I try debugging this with FF, and Firebug, stepping through the code - the page does not get reloaded, but the "Sign In" button does not get displayed either. If I remove the breakpoint in Firebug, then it goes back to looping through the page reload.

I am getting exactly the same with Chrome, but not with IE10 which loads the page but doesn't display the "Sign In" button.

For the unclaimed site, owner = '' when the call wiki.persona(owner); is made in static.html - that looks right.

As I am seeing different results depending on if I am running the dev tools, or not, I'm beginning to wonder if this could be a timing issue - or something wrong with the generated javascript?

Looking at the loggedInUser docs, it would seem that '' might be getting treated as undefined? Which would trigger a logout...

[ozten]

You're seeing this issue with my latest patch? This should be fixed now.

The login/logout events are geneated by navigator.id.watch when the Persona session and the wiki session don't agree. loggedInUser must be either null or a valid email address.

[paul90]

Sorry, ignore that - looks as if fetching from upstream is no longer fetching the tracking branches for the pull requests - not sure why I didn't spot that earlier - looks as if the last merge into my branch is from the 2nd.

Looks as if the recent Github for Windows update has changed the config - will sort this in the morning and the pull request branches are no longer getting fetched...

[paul90]

Just a quick update - managed to sort my git config issue.

The latest patch fixes the looping problem with a fresh site.

The only remaining issue I am seeing is with IE10 - the problem is that onmatch is not getting called, so the sign in button remains hidden - seem to be some related browserid issues - looks to be tied up with p3p issues.

[rynomad]

quick note: when merging upstream changes from the recent grunt pull request, git complains a lot. luckily, it's only about the compiled js, not the .coffee. running grunt build seems to sort everything out.

[WardCunningham]

Austin, Paul -- I really appreciate the effort you have put into Persona. I could see a collision coming with Nick and Christian's work on build process, which is client-side only at the moment. We've always thrown compiled javascript into the repo just to make installs easier. This may have been short sighted. Soon we will fix this by having a separate deployment mechanism. Until then, we won't worry too much about javascript thrashing back and forth with build system variations. Thanks.

[ozten]

Okay, I was using the wrong event callback. After switching to onready to show the auth UI, everything is dandy in IE 10.

Sorry about that. I think this patch is unblocked again, for review.

[paul90]

Probably need to go into the MDN wiki and split onready out of onmatch, didn't spot that event either.

Looking good - but, it would be a good idea to merge the latest changes to master into this branch - already done on my testing branch. The only conflict is the generated js, so run the grunt script to regenerate...

[paul90]

Looking back through the comments, I'm sure I commented on the vertical alignment of the Persona sign-in button. It would be good to tweak this, so it aligns better with the search box. So just a one-line css change.

[ozten]

Merged from master and tweaked CSS to align Search and Sign in button.

[ozten]

How are we feeling about this pull request? Anything I can do?

[WardCunningham]

Thank you both for all your work. When @ozten and I started this on hack night, I suggested we set both OpenID and Ruby implementations aside. Now I'm faced with what to do with all of the sites I'm hosting based on Ruby and OpenID. I'm sure you have left me with the raw material for an excellent solution. Help me think through what this might be.

[WardCunningham]

This work merged soothly on my laptop. @nrn helped me figure out how to configure git to remote all outstanding pull requests.

I'm surprised that the ruby and node versions continue to work for the same site. How can this be? I notice that both versions write identity information into status/open_id.identity. This is my email id when using node/persona. If I delete the file and then log in through ruby I can claim the site again with openId. Then I find an openId domain in the status/open_id.identity file. This makes sense. But, hey, the node/persona version still works. How can this be? Where does it keep my email address? Not in status/open_id.identity, I deleted that.

I'm trying to assess the work still required to coexist with ruby. @nrn suggests splitting the repos. It would be easy then to become more node-like in everything, including install and plugin distribution. Thoughts?

[paul90]

But, hey, the node/persona version still works. How can this be?

I have noticed that if you delete status/open_id.identity while the server is still running it looks very much as if the contents has been cached somewhere - as the server carries on as if it is still there. So, if you have both servers running it might be possible to have them both think you are the owner with different credentials.

Not too sure about splitting the repos, as the client code is shared. Though splitting the plugins out into their own repos will probably help encourage the development of more.

[ozten]

I've submitted d6f5519 which renames the status file to persona.identity.

I think not doing this earlier was a mistake.

Now you can run a wiki on Ruby with OpenID, then switch it to Node.js with Persona. The data directory can happily coexist with open_id.identity.

[WardCunningham]

Thank you. I apologize for the small and careful steps when there is so little at stake. It seems now that users of a ruby site can coexist with users of a node site. The owner will need to identify twice if he/she wants to use both.

[WardCunningham]

We've created a new repo for the node.js server implementation. It will authenticate with Persona using this code you have so kindly contributed. We've published this as an npm package using directory structure much more like one would expect of a node program. If this works well, we expect it to become the reference implementation for federated wiki. It is my intention to close this pull request once the health of the new repository has been established.

I have two regrets. First that it has taken me so long to incorporate this code anywhere. And second that we lose the contribution history in the move. This repo has been fattened with generated javascript. The new repo will be developer focused with coffeescript only while the generated javascript will be published in the npm module for simple deployment.

I look forward to your ongoing contributions to which ever repo you find most useful. Thanks again.

[ozten]

This is great news!

A new focused repo makes a lot of sense.

Congrats on the new repo!

[WardCunningham]

If there are no complaints, #398 will replace the client-side javascript with the wiki-client npm module including the persona modifications included in this pull request. This has been the long way around for sure for the node version, which we will also remove from this repo making it exclusively server-side ruby for federated wiki. At this point we'll close this request.