Sounds good - I think the main reason I didn't do this was because I changed the route to /login/:provider[/:redirect] - so the provider isn't optional here (without overriding the route, anyway). I just liked the format of this URL better, but in the end it doesn't matter much and backwards compatibility is probably more important.

I could change it to /login/:redirect[/:provider], which would allow the provider to remain optional, so backwards compatibility could be retained. Do you think that is a good approach?