Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Bug: Unauthorized Access Attempt Crashes App #86

Closed
baer opened this issue Sep 4, 2015 · 9 comments
Closed

Bug: Unauthorized Access Attempt Crashes App #86

baer opened this issue Sep 4, 2015 · 9 comments
Labels

Comments

@baer
Copy link
Contributor

baer commented Sep 4, 2015

This view comes back as undefined and crashes the app: https://github.com/stormpath/stormpath-express/blob/ec77ebee48a4cc790856b08dd6e9e8ef15bb120a/lib/authentication.js#L59

  app.use(stormpath.init(app, {
    website: true,
    client: {
      apiKey: {
        id: config.stormpath.id,
        secret: config.stormpath.secret
      }
    },
    application: {
      href: config.stormpath.url
    }
  }));
  app.get('/', stormpath.groupsRequired(['employee']), function(req, res) {
    res.sendFile('app.html', {root: config.root});
  });

stompath-express#2.0.1
node 0.12.7

@rdegges
Copy link
Contributor

rdegges commented Sep 4, 2015

Working on this now.

@rdegges rdegges added the bug label Sep 4, 2015
@baer
Copy link
Contributor Author

baer commented Sep 6, 2015

Thanks @rdegges!

@rdegges
Copy link
Contributor

rdegges commented Sep 8, 2015

Got stuck writing tests for this on Friday -- continuing where I left off now =)

My new strategy with issues here is to first:

  • Write full integration tests to reproduce.
  • Fix bugs.
  • Pass all tests.
  • Write / update all necessary docs.
  • Submit PR to resolve.
  • Cut release.

@baer
Copy link
Contributor Author

baer commented Sep 8, 2015

I ❤️ that phrase: "Got stuck writing tests." Thanks for looking into this! If I understood more about express middleware I'd try to just PR it myself :/

@rdegges
Copy link
Contributor

rdegges commented Sep 8, 2015

Hah, well -- I've been trying to improve test coverage. My goal is to dramatically improve the quality of this library ASAP. So, having 100% coverage, ensuring all bugs are 100% squashed, etc. Takes a bit longer to fix stuff this way, but makes code wayy better ^^

@rdegges
Copy link
Contributor

rdegges commented Sep 8, 2015

All these repos will soon be at 0 open issues / PRs >:)

@rdegges rdegges closed this as completed in ccf3ee9 Sep 8, 2015
@rdegges
Copy link
Contributor

rdegges commented Sep 8, 2015

Ok, this is all fixed / good to go. There is still some work left to handle customizing the view, but I'm going to handle that as a separate github issue because it doesn't really make sense to include it in this. I'm cutting a new release with this fix included right now (2.0.3).

@rdegges
Copy link
Contributor

rdegges commented Sep 8, 2015

Release cut!

@baer
Copy link
Contributor Author

baer commented Sep 8, 2015

Awesome! As soon as registration works again (#87) I'mma pull this into my project!

🤘 🤘 🤘

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants