Add Eventbrite Authentication #38

indcoder · 2017-07-19T22:47:02Z

No description provided.

Upstream Upate with LinkedIn

In production its best to go with a docker image that is nailed down to its patch number , very useful duing debugging

Instagram Addition

Added Eventrbite OAuth 2.0 to the list of authentication mechanism

The merge conflict resolution added an extra space before the square bracket which tripped standard js

coveralls · 2017-07-19T22:56:04Z

Coverage increased (+0.04%) to 99.27% when pulling ad82fc5 on MumbaiHackerspace:master into f75f2d8 on lipp:master.

lipp · 2017-07-23T17:42:20Z

src/strategies/eventbrite.js

+        username,
+        name: displayName,
+        provider: 'eventbrite',
+        email: emails[0].value


up to now, email is not included in public profile, since it it considered private information. the profile is not http-only and thus is more likely to be subject to an evil script / "attack".

You are right ...I just realized that we dont require the email per se but rather we can have the Eventbrite userId in the username.

lipp · 2017-07-23T17:45:18Z

index.js

@@ -4,6 +4,8 @@ const cookieParser = require('cookie-parser')
 const expressSession = require('express-session')
 const MemoryStore = require('session-memory-store')(expressSession)

+require('dotenv').config()


not sure if we really should use it... do you have some hoster/deployment scenario which honors this way to pass env?

The use is mostly for local deployment or when not using Docker ....especially when we are deploying directly to cloud like EC2 instances, we can just deploy .env file along with the production bundle

lipp · 2017-07-23T17:46:00Z

besides comments 💯

Since the email stored is not secure and could be misused, we should not use email but rather store Eventbrite user id Closes lipp#39

coveralls · 2017-07-26T15:45:25Z

Coverage increased (+0.04%) to 99.27% when pulling a2e12f3 on MumbaiHackerspace:master into f75f2d8 on lipp:master.

lipp · 2017-07-23T17:48:11Z

package.json

@@ -35,6 +36,7 @@
    "cookie-parser": "^1.4.3",
    "express": "^4.14.0",
    "express-session": "^1.14.2",
+    "dotenv-extended": "^2.0.1",


why do we need extended?

do we really need this? isn't dotenv enough?

lipp · 2017-07-28T17:28:41Z

example/nextjs/pages/index.js

@@ -39,7 +39,7 @@ const Index = ({profile, origin}) => (
      <h2 className='subtitle is-4'>
        Stateless authentication microservice for
        <ul>
-          {['Twitter', 'Facebook', 'Google', 'GitHub', 'Reddit', 'LinkedIn', 'Instagram'].map(name => <Item name={name} key={name} />)}
+          {['Twitter', 'Facebook', 'Google', 'GitHub', 'Reddit', 'LinkedIn', 'Instagram', 'Eventbrite'].map(name => <Item name={name} key={name} />)}


I will not feature every login strategy on the demo page. Could you remove this?

I am in two minds about this

If there is a particular strategy thats implemented within the login-with mircoservice, it should follow that there should be a button that will enable that authentication.

It is unreasonable to expect everyone to enable buttons for ALL auth strategies

=> The auth buttons that are needed to be enabled or disabled should be configurable.
Also the URL in the example is hardcoded for it to make a call to login.login-with.com for auth calls, this too should be configurable because it will help others to test their added strategy on their own domain before pushing it unto upstream repo

The login-with.com page (and the related demo code) is not meant to be deployed by anyone else.

The respective user / developer can configure which strategies to use via environment variables.

To test a specific strategy, you can directly connect (from browser) to your auth endpoint (e.g. auth.login-with.com/twitter or auth.your-service.com/eventbrite)

But on the login-with site, no one will know ALL the strategies that have been implemented. I thinks its in the best interest of this OSS project to advertise all the strategies it supports.
PS: I do not have a skin in the game nor do i rep Eventbrite. Its just that we run a OSS meetup group and we use EB for our checkins and we are building an OSS app to do this automatically.

Also test not just the API but test it as a consumable from an app[which is its actual purpose] which the nextjs is ideal...since we can compare the behaviour across strategies.

lipp · 2017-07-28T17:28:45Z

example/nextjs/pages/login.js

@@ -26,7 +26,7 @@ const Login = ({profile, origin}) => {
  const redirect = encodeURIComponent(origin + '/')
  return (
    <div className='section'>
-      { !profile && ['Twitter', 'Google', 'GitHub', 'Reddit', 'Facebook', 'LinkedIn', 'Instagram'].map(service => (
+      { !profile && ['Twitter', 'Google', 'GitHub', 'Reddit', 'Facebook', 'LinkedIn', 'Instagram', 'Eventbrite'].map(service => (


I will not feature every login strategy on the demo page. Could you remove this?

lipp · 2017-07-28T17:29:01Z

README.md

@@ -87,8 +88,8 @@ must be: `https://login.yourdomain.com/facebook/callback`
 You need to create your own LinkedIn OAuth2 application. If `LW_SUBDOMAIN=login.yourdomain.com` your Authorization callback URL
 must be: `https://login.yourdomain.com/linkedin/callback`

- `LW_LINKEDIN_CLIENTID` - Your Google Client ID


Good watch! THX

coveralls · 2017-07-30T19:25:34Z

Coverage increased (+0.04%) to 99.27% when pulling 958c24d on MumbaiHackerspace:master into f75f2d8 on lipp:master.

lipp · 2017-07-31T11:27:02Z

we can keep dotenv in... just wanted to know why dotenv-extended is required.

… file

Modified environment variable names as per the eventbrite.js strategy…

coveralls · 2017-08-01T14:23:56Z

Coverage increased (+0.04%) to 99.27% when pulling 9313f6f on MumbaiHackerspace:master into f75f2d8 on lipp:master.

indcoder added 11 commits July 7, 2017 21:32

Merge pull request #1 from lipp/master

74dcb91

Upstream Upate with LinkedIn

Added Eventrbite credentials

008d614

Upstream changes for LinkedIn

55e80e0

Added keyword for eventbrite

11a8f1f

Removing Google references in LinkedIn block

093f597

build(Docker): Ensure the Docker alpine image is very specific

e5b1d2a

In production its best to go with a docker image that is nailed down to its patch number , very useful duing debugging

Merge pull request #2 from lipp/master

769022a

Instagram Addition

build(git): Exclude next.js generated output from git

ef400f6

feat(project): Add Eventbrite authentication

936a554

Added Eventrbite OAuth 2.0 to the list of authentication mechanism

Merge Commit of Instagram _+ Eventbrite addition

2916108

style(login): Remove extra space from the ending bracket

ad82fc5

The merge conflict resolution added an extra space before the square bracket which tripped standard js

lipp requested changes Jul 23, 2017

View reviewed changes

fix(package): Remove email as part of the profile stored

a2e12f3

Since the email stored is not secure and could be misused, we should not use email but rather store Eventbrite user id Closes lipp#39

lipp requested changes Jul 28, 2017

View reviewed changes

refactor(package): Remove dotenv references

958c24d

Maninderjit Bindra and others added 2 commits August 1, 2017 19:45

Modified environment variable names as per the eventbrite.js strategy…

faa3190

… file

Merge pull request #3 from maniSbindra/modify-readme

9313f6f

Modified environment variable names as per the eventbrite.js strategy…

lipp mentioned this pull request Aug 3, 2017

Add Eventbrite authentication #37

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Eventbrite Authentication #38

Add Eventbrite Authentication #38

indcoder commented Jul 19, 2017

coveralls commented Jul 19, 2017

lipp Jul 23, 2017

indcoder Jul 24, 2017

lipp Jul 23, 2017

indcoder Jul 24, 2017

lipp Jul 28, 2017

lipp commented Jul 23, 2017

coveralls commented Jul 26, 2017

lipp Jul 23, 2017

lipp Jul 28, 2017

lipp Jul 28, 2017

indcoder Jul 30, 2017

lipp Jul 31, 2017

indcoder Jul 31, 2017

lipp Jul 28, 2017

lipp Jul 28, 2017

coveralls commented Jul 30, 2017

lipp commented Jul 31, 2017

coveralls commented Aug 1, 2017

Add Eventbrite Authentication #38

Are you sure you want to change the base?

Add Eventbrite Authentication #38

Conversation

indcoder commented Jul 19, 2017

coveralls commented Jul 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lipp commented Jul 23, 2017

coveralls commented Jul 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jul 30, 2017

lipp commented Jul 31, 2017

coveralls commented Aug 1, 2017