feat: call gsuite-wrapper on key moments #183

linuxbandit · 2020-11-22T21:50:54Z

we can change superagent if needed. Some things are still quite rough and only placeholders.

The things that needs work are:

transliteration
unique name checking
workflows checking
create a user on gsuite when the members list are uploaded (danger zone)

middlewares/campaigns.js

middlewares/members.js

serge1peshcoff · 2020-11-23T08:19:26Z

general comment:

check out what eslint is complaining about
some tests are failing apparently
can we add some tests for gsuite wrapper? to stay on 100% coverage

linuxbandit · 2020-11-23T15:55:40Z

tests for gsuite wrapper are already there, i don't pipeline them because I want control over them (since it involves many rapid hitting google endpoints and creating/deleting users etc, I want to be careful of the quota)

serge1peshcoff · 2020-11-23T15:57:30Z

on tests I mean the following: mocking the request to gsuite-wrapper and checking if it works under different conditions (it returns error, garbage, valid data etc.), I have something similar for mailer so check this out as well

WikiRik · 2020-11-25T21:02:29Z

Workflow has been checked, everything that needs to be done is the following;

Implement everything from TODOs
Change superagent to request and request-promise-native
Change payload generation to new file lib/gsuite.js
Add tests, mock requests to gsuite-wrapper as is also done for requests to mailer
Add validation to the gsuite_id that checks if it does not exist in either body, circle or user

For the third point, see comment of @serge1peshcoff
"we have the payload generation and actual query inside the middleware, which I think doesn't belong here. instead, we can create a module (like lib/gsuite.js) which will contain functions that are wrappers for gsuite-wrapper requests.
check out how I call functions from mailer.js for the idea"

WikiRik · 2020-11-25T21:24:59Z

On the transliteration of first/last names, check the NAME_REGEX in models/User.js

WikiRik · 2020-11-26T17:07:40Z

middlewares/campaigns.js

+                familyName: req.body.last_name
+            },
+            secondaryEmail: req.body.email,
+            password: crypto.createHash('sha1').update(JSON.stringify(req.body.password)).digest('hex'),


@linuxbandit as per https://stackoverflow.com/questions/24925304/in-googles-directory-api-what-hash-format-is-required-when-crypt-is-provided we can also use SHA-512, can you try that in GSuiteWrapper?

@serge1peshcoff this is what needs to be changed in GSuiteWrapper

ah okay, haven't seen this comment somehow

That's because we use the same function in two places. So it's good if we move it to a class where we can just define it once and call it twice

WikiRik · 2021-04-12T18:26:17Z

middlewares/bodies.js

@@ -52,6 +52,8 @@ exports.createBody = async (req, res) => {
        return errors.makeForbiddenError(res, 'Permission global:create:body is required, but not present.');
    }

+    // TODO: if antenna, contact antenna or contact, then create GSuite account


This can also be done later

WikiRik · 2021-04-12T18:26:21Z

middlewares/bodies.js

@@ -65,6 +67,8 @@ exports.updateBody = async (req, res) => {
        return errors.makeForbiddenError(res, 'Permission update:body is required, but not present.');
    }

+    // TODO: if antenna, contact antenna or contact, then update GSuite account if needed


This can also be done later

WikiRik · 2021-04-12T18:26:27Z

middlewares/bodies.js

@@ -89,6 +93,7 @@ exports.setBodyStatus = async (req, res) => {
        await BodyMembership.destroy({ where: { body_id: req.currentBody.id }, transaction: t });
        await Circle.destroy({ where: { body_id: req.currentBody.id }, transaction: t });
        await Payment.destroy({ where: { body_id: req.currentBody.id }, transaction: t });
+        // TODO: suspend GSuite account (if attached)


This can also be done later

WikiRik · 2021-04-12T18:26:31Z

middlewares/bodies.js

@@ -122,6 +127,7 @@ exports.createMember = async (req, res) => {
        body_id: req.currentBody.id
    });

+    // TODO: create active GSuite account


This can also be done later

Nevermind, this should create a GSuite account for new users. That should be implemented at the start already

WikiRik · 2021-04-12T18:40:22Z

middlewares/members.js

@@ -63,6 +64,18 @@ exports.updateUser = async (req, res) => {
    }

    await req.currentUser.update(req.body, { fields: constants.FIELDS_TO_UPDATE.USER.UPDATE });
+
+    // TODO: update first/late name to GSuite account (if there is an account attached)


I mention it here, but it goes for the whole file; not every user will have a GSuite account attached (mostly older accounts) so we also need to check that we only do things if an account is attached

serge1peshcoff · 2021-07-16T07:33:28Z

lib/cron.js

@@ -1,4 +1,5 @@
 const cron = require('node-cron');
+const superagent = require('superagent');


you don't need superagent as an extra dependency for it, we have request and request-promise-native already

I know, Fabri added it before and I just kept it there. It will be replaced before it will be merged

PR is still WIP

serge1peshcoff · 2021-07-16T07:36:29Z

middlewares/campaigns.js

@@ -36,6 +36,7 @@ exports.registerUser = async (req, res) => {
        });

        // TODO: add uniqueness check; transliterate gsuiteEmail in case of umlauts or other special characters
+        // TODO: probably move this elsewhere since not all MyAEGEE users need a GSuite account
        const gsuiteEmail = req.body.first_name + '.' + req.body.last_name + '@' + constants.GSUITE_DOMAIN;
        const payload = {


better to do it this way: have a file to contain all gsuite functions (or even better, make it a class, like GSuiteClient, create instance of it and assign it to something like req.gsuiteClient), then you can reference this class instead of having this logic in a middleware itself

That's a great idea

serge1peshcoff · 2021-07-16T07:36:51Z

middlewares/members.js

-    // await superagent.post('gsuite-wrapper:8084/accounts?SETPASS')
+    if (req.currentUser.gsuite_id) {
+        const payload = {
+            password: crypto.createHash('sha1').update(JSON.stringify(req.body.password)).digest('hex')


github complains here, is it okay? if it is, can it be muted?

This needs to be fixed in GSuiteWrapper itself first, we can temporarily mute it if we merge this before

Also fixed linter and installed superagent. Superagent will have to be removed to use request

This is not in the scope of the current issues, so I removed it from here

Not all merge conflicts were resolved correctly

linuxbandit requested a review from serge1peshcoff November 22, 2020 21:50

linuxbandit marked this pull request as draft November 22, 2020 21:51

linuxbandit mentioned this pull request Nov 22, 2020

fix(gsuite): add gsuite model #184

Merged

linuxbandit requested a review from WikiRik November 22, 2020 21:57

serge1peshcoff reviewed Nov 22, 2020

View reviewed changes

middlewares/campaigns.js Outdated Show resolved Hide resolved

serge1peshcoff reviewed Nov 22, 2020

View reviewed changes

middlewares/campaigns.js Outdated Show resolved Hide resolved

serge1peshcoff reviewed Nov 22, 2020

View reviewed changes

middlewares/campaigns.js Outdated Show resolved Hide resolved

serge1peshcoff reviewed Nov 22, 2020

View reviewed changes

middlewares/members.js Outdated Show resolved Hide resolved

WikiRik reviewed Nov 26, 2020

View reviewed changes

WikiRik reviewed Apr 12, 2021

View reviewed changes

WikiRik force-pushed the feat/add-calls-to-gsuite-wrapper branch from 01d41c3 to e23e889 Compare July 15, 2021 21:43

serge1peshcoff reviewed Jul 16, 2021

View reviewed changes

linuxbandit and others added 6 commits October 14, 2021 12:24

feat: call gsuite-wrapper on key moments

2f0dc24

fix(gsuite): add TODOs for everything that has to be done

8652c3f

Also fixed linter and installed superagent. Superagent will have to be removed to use request

chore(gsuite): remove TODO for group creation for non-locals

7300fe1

This is not in the scope of the current issues, so I removed it from here

chore(gsuite): rebased on master

eeac395

chore(gsuite): fixed rebase

7409058

Not all merge conflicts were resolved correctly

fix(gsuite): implemented few TODOs

c4f0fab

WikiRik force-pushed the feat/add-calls-to-gsuite-wrapper branch from 9b38d70 to c4f0fab Compare October 14, 2021 10:24

chore(request): replace superagent with request

b6490bf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: call gsuite-wrapper on key moments #183

feat: call gsuite-wrapper on key moments #183

linuxbandit commented Nov 22, 2020 •

edited

serge1peshcoff commented Nov 23, 2020

linuxbandit commented Nov 23, 2020

serge1peshcoff commented Nov 23, 2020

WikiRik commented Nov 25, 2020 •

edited

WikiRik commented Nov 25, 2020

WikiRik Nov 26, 2020

WikiRik Jul 16, 2021

serge1peshcoff Jul 16, 2021

WikiRik Jul 16, 2021 •

edited

serge1peshcoff Jul 16, 2021

WikiRik Apr 12, 2021

WikiRik Apr 12, 2021

WikiRik Apr 12, 2021

WikiRik Apr 12, 2021

WikiRik Apr 12, 2021

WikiRik Apr 12, 2021

serge1peshcoff Jul 16, 2021

WikiRik Jul 16, 2021

WikiRik Jul 16, 2021

serge1peshcoff Jul 16, 2021

WikiRik Jul 16, 2021

serge1peshcoff Jul 16, 2021

WikiRik Jul 16, 2021

		@@ -1,4 +1,5 @@
		const cron = require('node-cron');
		const superagent = require('superagent');

feat: call gsuite-wrapper on key moments #183

Are you sure you want to change the base?

feat: call gsuite-wrapper on key moments #183

Conversation

linuxbandit commented Nov 22, 2020 • edited

serge1peshcoff commented Nov 23, 2020

linuxbandit commented Nov 23, 2020

serge1peshcoff commented Nov 23, 2020

WikiRik commented Nov 25, 2020 • edited

WikiRik commented Nov 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WikiRik Jul 16, 2021 • edited

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

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

linuxbandit commented Nov 22, 2020 •

edited

WikiRik commented Nov 25, 2020 •

edited

WikiRik Jul 16, 2021 •

edited