Hi,

@jankapunkt
@jorenvandeweyer
@Uzlopak

I have updated the pr, so please check whether I have understood the comments, or something is still missing.
Btw, I have also found and fixed a bug in AbstractGrantType.prototype.validateScope(): it did not return a promise in all cases (but now it does).

Regards

To be honest: I dont like it.

I expected a minimal change like this:

AuthorizationCodeGrantType.prototype.saveToken = function(user, client, authorizationCode, requestedScope) {
 const scope = await this.validateScope(user, client, requestedScope);
 const fns = [
 this.generateAccessToken(client, user, scope),
 this.generateRefreshToken(client, user, scope),
 this.getAccessTokenExpiresAt(),
 this.getRefreshTokenExpiresAt()
 ];
 return Promise.all(fns)
 .bind(this)
 .spread(function(accessToken, refreshToken, accessTokenExpiresAt, refreshTokenExpiresAt) {
 const token = {
 accessToken: accessToken,
 authorizationCode: authorizationCode,
 accessTokenExpiresAt: accessTokenExpiresAt,
 refreshToken: refreshToken,
 refreshTokenExpiresAt: refreshTokenExpiresAt,
 scope: scope
 };
 return promisify(this.model.saveToken, 3).call(this.model, token, client, user);
 });
};

I also think you can just use await like @Uzlopak suggests. @jwerre is already rewriting the code base to use await everywhere else.

I would wait until it's done with the rewrite and currently focus on integrity for the current state of code. I wil review later today

Hi,

I have re-written by using await, but now 3 tests fail.
I am not able to fix them... can someone help please?

Regards

I like your change. Good work!

I would suggest to delete the failing tests, but this could be an inconsistency throughout the codebase. We should wait till everywhere is async await and then merge your change.

@FStefanni no worries we will check out what's going on here

@Uzlopak @HappyZombies @jorenvandeweyer should this also be added to 4.2.0? I think it's not really "breaking" so I would add it to the release.