Skip to content

Conversation

@jiachen247
Copy link
Contributor

This is a PR to port authentication from IVLE to LumiNUS.
closes #337

As discussed with Prof Martin, to remove announcements from cadet.

@jiachen247 jiachen247 requested a review from indocomsoft April 17, 2019 18:16
@coveralls
Copy link

coveralls commented Apr 17, 2019

Pull Request Test Coverage Report for Build 2444

  • 24 of 25 (96.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 97.479%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/cadet/accounts/luminus.ex 16 17 94.12%
Files with Coverage Reduction New Missed Lines %
lib/cadet_web/controllers/auth_controller.ex 1 97.22%
Totals Coverage Status
Change from base Build 2441: -0.2%
Covered Lines: 812
Relevant Lines: 833

💛 - Coveralls

@jiachen247
Copy link
Contributor Author

Accompanying PR on the frontend can be found here!
source-academy/frontend#534

@jiachen247 jiachen247 requested a review from ning-y April 17, 2019 18:30
@jiachen247
Copy link
Contributor Author

Object.defineProperty(Task.prototype, "isPast", {
        get: function () {
            return __WEBPACK_IMPORTED_MODULE_3_moment__().isAfter(this.endDate);
        },
        enumerable: true,
        configurable: true
    });

taken from https://luminus.nus.edu.sg/main.d1c031e4d375a32b95d6.bundle.js

Looks like the luminus web client parses and checks the end date to see if the module is over. looks like we have to do the same!

@indocomsoft
Copy link
Member

Sure. For fluminus I checked against the current term, but that's 1 additional API call that luminus backend might not be able to afford. Let's do that then (check end date).

@jiachen247
Copy link
Contributor Author

jiachen247 commented May 10, 2019

@indocomsoft Edit: sorry yessss youre right!!

@jiachen247
Copy link
Contributor Author

jiachen247 commented May 10, 2019

Object.defineProperty(CourseModule.prototype, "isStudent", {
        get: function () {
            return this.access && (this.access.accessRead &&
                !this.access.accessCreate &&
                !this.access.accessDelete &&
                !this.access.accessFull &&
                !this.access.accessSettingsRead &&
                !this.access.accessSettingsUpdate &&
                !this.access.accessUpdate);
        },
        enumerable: true,
        configurable: true
    });
    Object.defineProperty(CourseModule.prototype, "isReadManager", {
        get: function () {
            return this.access && (this.access.accessRead &&
                !this.access.accessCreate &&
                !this.access.accessDelete &&
                !this.access.accessFull &&
                this.access.accessSettingsRead &&
                !this.access.accessSettingsUpdate &&
                !this.access.accessUpdate);
        },
        enumerable: true,
        configurable: true
    });
    Object.defineProperty(CourseModule.prototype, "isManager", {
        get: function () {
            return this.access && (this.access.accessRead &&
                this.access.accessCreate &&
                this.access.accessDelete &&
                !this.access.accessFull &&
                this.access.accessSettingsRead &&
                this.access.accessSettingsUpdate &&
                this.access.accessUpdate);
        },
        enumerable: true,
        configurable: true
    });
    Object.defineProperty(CourseModule.prototype, "isOwner", {
        get: function () {
            return this.access && (this.access.accessRead &&
                this.access.accessCreate &&
                this.access.accessDelete &&
                this.access.accessFull &&
                this.access.accessSettingsRead &&
                this.access.accessSettingsUpdate &&
                this.access.accessUpdate);
        },
        enumerable: true,
        configurable: true
    });

And this is how roles and permissions are assigned and handled under the hood on the luminus web client.

@martin-henz how do you suggest we go around mapping the permissions?

I suggest we do

Student -> student
Manager & ReadManager -> staff (we can use this for the TAs)
Owner & Co Ownder -> admin role

is this advisable? this would be

@jiachen247 jiachen247 requested a review from indocomsoft May 12, 2019 09:43
@jiachen247 jiachen247 requested a review from indocomsoft May 19, 2019 09:26
@jiachen247
Copy link
Contributor Author

After talking to @indocomsoft we agreed following map.get/3 with a default value would be the cleanest way to do Luminus.fetch_luminus_token/2.

https://hexdocs.pm/elixir/Map.html#get/3

@jiachen247 jiachen247 requested a review from rrtheonlyone June 5, 2019 09:42
Copy link
Contributor

@rrtheonlyone rrtheonlyone left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@jiachen247 jiachen247 dismissed indocomsoft’s stale review June 5, 2019 10:40

sorry julius haha

@jiachen247 jiachen247 merged commit 201dcae into master Jun 5, 2019
@jiachen247 jiachen247 deleted the luminus-auth branch June 5, 2019 10:40
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.

IVLE is being deprecated next semester

5 participants