Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use authCode from config again #150

Merged
merged 2 commits into from Dec 8, 2019
Merged

Use authCode from config again #150

merged 2 commits into from Dec 8, 2019

Conversation

robbertkl
Copy link
Contributor

After the switch to magister-openid, the given authCode was no longer used, since magister-openid had its own (outdated) authCode. This has since been fixed in v0.1.4 with a breaking change requiring to pass along the authCode.

This PR fixes the backwards incompatibility by requiring at least 0.1.4 and passes the authCode along to magister-openid.

The reason we need to be able to pass an authCode is that magisterjs-authcode is no longer updated, see simplyGits/magisterjs-authcode#4. Any idea why that is? I had to create my own automatic authcode update tool (https://github.com/robbertkl/magister-authcode).

idiidk
idiidk previously approved these changes Dec 5, 2019
@idiidk
Copy link
Collaborator

idiidk commented Dec 5, 2019

If you update the magister-openid version to 0.1.5 in this PR, the whole authentication side of the project should work again.

@robbertkl
Copy link
Contributor Author

@idiidk Done!

@robbertkl
Copy link
Contributor Author

Just tested & confirmed: the fixes @idiidk applied to magister-openid together with this PR fixes the entire auth flow.

Could you also push a release (another alpha is fine) so @latest works again? Thanks!

@lieuwex
Copy link
Member

lieuwex commented Dec 8, 2019

Sorry for the late reply, usual university stuff. With a bit of Sinterklaas mixed in.

What I don't understand: 0.1.4 was released 2 months ago but since that was a breaking change didn't that break magisterjs?

Any idea why that is?

I was (as many people are to my knowledge) contacted my Magister and asked to shut the "circumvention" (authcode retrieval, whatever you want to call it) down. Magister.js is now to be used with an authcode provided by the user, which you can get on several ways. But I just won't provide any.

I would still get the official API documentation as to keep the package up-to-date all the time, since it can of course be used in officially certified projects, and since I don't have a Magister account so I can't test the package and dissect the API for myself. But they haven't still sent it to me after months.

If you find some things suboptimal with the state of magisterjs I invite you to fork it, so you don't have to wait on my slow replies all the time ;)

Copy link
Member

@lieuwex lieuwex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@lieuwex lieuwex merged commit 452c92a into simplyGits:master Dec 8, 2019
@lieuwex
Copy link
Member

lieuwex commented Dec 8, 2019

A new version has been published: https://github.com/simplyGits/MagisterJS/releases/tag/2.0.0-alpha.13

@robbertkl
Copy link
Contributor Author

Thanks @lieuwex !

What I don't understand: 0.1.4 was released 2 months ago but since that was a breaking change didn't that break magisterjs?

Yep, it did break MagisterJS (master branch), but only if you installed after 0.1.4 was released. If you did not update your (sub)dependencies, it would still use 0.1.3 or 0.1.2 and still work.

Also, this was only true for an install from master. The latest npm release (then 2.0.0-alpha.12) did not yet use magister-openid, so it only broke a few days ago when the challenge URLs were changed from /challenge to /challenges (see idiidk/magister-openid#1).

I was (as many people are to my knowledge) contacted my Magister and asked to shut the "circumvention" (authcode retrieval, whatever you want to call it) down. Magister.js is now to be used with an authcode provided by the user, which you can get on several ways. But I just won't provide any.

So if I understand correctly, the Magister website uses its own authcode, that changes periodically (to prevent abuse) and you can get your own (static) authcode from Iddink? Are they a bit easy/quick to give out authcodes? For stuff like simple personal automation projects instead of general purpose Magister-based applications.

If you find some things suboptimal with the state of magisterjs I invite you to fork it, so you don't have to wait on my slow replies all the time ;)

I used my own fork in the meantime, but will now switch back to the npm release.

@lieuwex
Copy link
Member

lieuwex commented Dec 9, 2019

Ah I see, thanks the for explanation.

Are they a bit easy/quick to give out authcodes?

I don't know, Their thought was that if you've built an app you can show it to them and they would check if it has enough security considerations. If that's the case they would give you permission to use their API.

But I've tried to convince them that his is a chicken-and-egg problem, since you'd need API access first to create an app to show.
They seemed convinced and seemed to like the approach of giving free access to your own account first.

I can't tell if they've gone through with that plan or not, you should contact them.
I've hope that if you have a good usecase and it's personal they will give you access.

@pascal541 pascal541 mentioned this pull request Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants