Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Removes instances of "any" and unnecessary type casts #236

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Removes instances of "any" and unnecessary type casts #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f37642
feat: removes instances of any
ma-petrovic Oct 12, 2019
c618fe4
feat: removes unnecessary type casts
ma-petrovic Oct 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/config/passport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import passport from "passport";
import passportLocal from "passport-local";
import passportFacebook from "passport-facebook";
import _ from "lodash";

import { Req } from "../interfaces/req";
// import { User, UserType } from '../models/User';
import { User, UserDocument } from "../models/User";
import { Request, Response, NextFunction } from "express";

const LocalStrategy = passportLocal.Strategy;
const FacebookStrategy = passportFacebook.Strategy;

passport.serializeUser<any, any>((user, done) => {
passport.serializeUser<UserDocument, UserDocument["id"]>((user, done) => {
done(undefined, user.id);
});

Expand Down Expand Up @@ -66,15 +66,15 @@ passport.use(new FacebookStrategy({
callbackURL: "/auth/facebook/callback",
profileFields: ["name", "email", "link", "locale", "timezone"],
passReqToCallback: true
}, (req: any, accessToken, refreshToken, profile, done) => {
}, (req: Req, accessToken, refreshToken, profile, done) => {
if (req.user) {
User.findOne({ facebook: profile.id }, (err, existingUser) => {
if (err) { return done(err); }
if (existingUser) {
req.flash("errors", { msg: "There is already a Facebook account that belongs to you. Sign in with that account or delete it, then link it with your current account." });
done(err);
} else {
User.findById(req.user.id, (err, user: any) => {
User.findById(req.user.id, (err, user: UserDocument) => {
if (err) { return done(err); }
user.facebook = profile.id;
user.tokens.push({ kind: "facebook", accessToken });
Expand All @@ -100,7 +100,7 @@ passport.use(new FacebookStrategy({
req.flash("errors", { msg: "There is already an account using this email address. Sign in to that account and link it with Facebook manually from Account Settings." });
done(err);
} else {
const user: any = new User();
const user = new User();
user.email = profile._json.email;
user.facebook = profile.id;
user.tokens.push({ kind: "facebook", accessToken });
Expand Down Expand Up @@ -130,10 +130,10 @@ export const isAuthenticated = (req: Request, res: Response, next: NextFunction)
/**
* Authorization Required middleware.
*/
export const isAuthorized = (req: Request, res: Response, next: NextFunction) => {
export const isAuthorized = (req: Req, res: Response, next: NextFunction) => {
const provider = req.path.split("/").slice(-1)[0];

const user = req.user as UserDocument;
const user = req.user;
if (_.find(user.tokens, { kind: provider })) {
next();
} else {
Expand Down
10 changes: 5 additions & 5 deletions src/controllers/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
"use strict";

import graph from "fbgraph";
import { Response, Request, NextFunction } from "express";
import { UserDocument } from "../models/User";
import { NextFunction, Request, Response } from "express";
import { Req } from "../interfaces/req";


/**
Expand All @@ -19,9 +19,9 @@ export const getApi = (req: Request, res: Response) => {
* GET /api/facebook
* Facebook API example.
*/
export const getFacebook = (req: Request, res: Response, next: NextFunction) => {
const user = req.user as UserDocument;
const token = user.tokens.find((token: any) => token.kind === "facebook");
export const getFacebook = (req: Req, res: Response, next: NextFunction) => {
const user = req.user;
const token = user.tokens.find((token) => token.kind === "facebook");
graph.setAccessToken(token.accessToken);
graph.get(`${user.facebook}?fields=id,name,email,first_name,last_name,gender,link,locale,timezone`, (err: Error, results: graph.FacebookUser) => {
if (err) { return next(err); }
Expand Down
31 changes: 18 additions & 13 deletions src/controllers/user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { IVerifyOptions } from "passport-local";
import { WriteError } from "mongodb";
import { check, sanitize, validationResult } from "express-validator";
import "../config/passport";
import { isKnownProvider } from "../interfaces/providers";
import { Req } from "../interfaces/req";

/**
* GET /login
Expand Down Expand Up @@ -130,7 +132,7 @@ export const getAccount = (req: Request, res: Response) => {
* POST /account/profile
* Update profile information.
*/
export const postUpdateProfile = (req: Request, res: Response, next: NextFunction) => {
export const postUpdateProfile = (req: Req, res: Response, next: NextFunction) => {
check("email", "Please enter a valid email address.").isEmail();
// eslint-disable-next-line @typescript-eslint/camelcase
sanitize("email").normalizeEmail({ gmail_remove_dots: false });
Expand All @@ -142,7 +144,7 @@ export const postUpdateProfile = (req: Request, res: Response, next: NextFunctio
return res.redirect("/account");
}

const user = req.user as UserDocument;
const user = req.user;
User.findById(user.id, (err, user: UserDocument) => {
if (err) { return next(err); }
user.email = req.body.email || "";
Expand All @@ -168,7 +170,7 @@ export const postUpdateProfile = (req: Request, res: Response, next: NextFunctio
* POST /account/password
* Update current password.
*/
export const postUpdatePassword = (req: Request, res: Response, next: NextFunction) => {
export const postUpdatePassword = (req: Req, res: Response, next: NextFunction) => {
check("password", "Password must be at least 4 characters long").isLength({ min: 4 });
check("confirmPassword", "Passwords do not match").equals(req.body.password);

Expand All @@ -179,7 +181,7 @@ export const postUpdatePassword = (req: Request, res: Response, next: NextFuncti
return res.redirect("/account");
}

const user = req.user as UserDocument;
const user = req.user;
User.findById(user.id, (err, user: UserDocument) => {
if (err) { return next(err); }
user.password = req.body.password;
Expand All @@ -195,8 +197,8 @@ export const postUpdatePassword = (req: Request, res: Response, next: NextFuncti
* POST /account/delete
* Delete user account.
*/
export const postDeleteAccount = (req: Request, res: Response, next: NextFunction) => {
const user = req.user as UserDocument;
export const postDeleteAccount = (req: Req, res: Response, next: NextFunction) => {
const user = req.user;
User.remove({ _id: user.id }, (err) => {
if (err) { return next(err); }
req.logout();
Expand All @@ -209,10 +211,13 @@ export const postDeleteAccount = (req: Request, res: Response, next: NextFunctio
* GET /account/unlink/:provider
* Unlink OAuth provider.
*/
export const getOauthUnlink = (req: Request, res: Response, next: NextFunction) => {
export const getOauthUnlink = (req: Req, res: Response, next: NextFunction) => {
const provider = req.params.provider;
const user = req.user as UserDocument;
User.findById(user.id, (err, user: any) => {
if (!isKnownProvider(provider)) {
return next(`Unrecognized provider '${provider}'`);
}
const user = req.user;
User.findById(user.id, (err, user) => {
if (err) { return next(err); }
user[provider] = undefined;
user.tokens = user.tokens.filter((token: AuthToken) => token.kind !== provider);
Expand Down Expand Up @@ -267,7 +272,7 @@ export const postReset = (req: Request, res: Response, next: NextFunction) => {
User
.findOne({ passwordResetToken: req.params.token })
.where("passwordResetExpires").gt(Date.now())
.exec((err, user: any) => {
.exec((err, user) => {
if (err) { return next(err); }
if (!user) {
req.flash("errors", { msg: "Password reset token is invalid or has expired." });
Expand Down Expand Up @@ -346,14 +351,14 @@ export const postForgot = (req: Request, res: Response, next: NextFunction) => {
});
},
function setRandomToken(token: AuthToken, done: Function) {
User.findOne({ email: req.body.email }, (err, user: any) => {
User.findOne({ email: req.body.email }, (err, user) => {
if (err) { return done(err); }
if (!user) {
req.flash("errors", { msg: "Account with that email address does not exist." });
return res.redirect("/forgot");
}
user.passwordResetToken = token;
user.passwordResetExpires = Date.now() + 3600000; // 1 hour
user.passwordResetToken = token.toString();
user.passwordResetExpires = new Date(Date.now() + 3600000); // 1 hour
user.save((err: WriteError) => {
done(err, token, user);
});
Expand Down
5 changes: 5 additions & 0 deletions src/interfaces/providers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const knownProviders = ["facebook"] as const;

export type KnownProvider = typeof knownProviders[number];

export const isKnownProvider = (provider: string): provider is KnownProvider => (knownProviders as readonly string[]).includes(provider);
6 changes: 6 additions & 0 deletions src/interfaces/req.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { Request } from "express";
import { UserDocument } from "../models/User";

export interface Req extends Request {
user: UserDocument;
}
Comment on lines +1 to +6
Copy link
Collaborator

Choose a reason for hiding this comment

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

I''m not sure if that is the best approach. In TS one usually augments missing changes using the type definition file (declaration merging), as we don't own the Request type:
E.g., for the userproperty on the Request:
https://github.com/microsoft/TypeScript-Node-Starter/pull/227/files

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the comment. Indeed, module augmentation may be the more idiomatic way to go about it, although I like the explicitness of wrapping ( extending ) a given type. Furthermore, it would be nice if the express's Request type were generic, similar to how RouteComponentProps accepts a parameter representing the type of the matched path params, however this is a topic for a different discussion. To get back to the main story line, why wasn't #227 merged yet ( #236 could build off of it )?