feat(middleware): Introduce IP Restriction Middleware

[nakasyou]

Recreated #2807

---

I created IP Limit Middleware.

You can limit request by IP Address.

For example, you can limit request, this server accepts local-only requests:

import { Hono } from 'hono'
import { ipLimit } from 'hono/ip-limit'
import { getConnInfo } from 'hono/...'

const app = new Hono()

app.use('*', ipLimit(getConnInfo, {
  deny: [],
  allow: ['127.0.0.1', '::1']
}))
app.get('/', c => c.text('Hello world!'))

deny takes precedence over allow.

Rules supported some syntax:

Title	example of IPv4	example of IPv6
static	0.0.0.0	::1
CIDR	192.168.2.1/24	abcd::ef01/64
Wildcard	192.*.2.*

The author should do the following, if applicable

• Add tests
• Run tests
• bun denoify to generate files for Deno
• bun run format:fix && bun run lint:fix to format the code

[EdamAme-x]

I think it would be good to have * automatically when allow is empty or undefined.
And I can also suggest a middleware called request-limit based on this.
nakasyou#1

[nakasyou]

Hi @EdamAme-x, thank you for comment and PR.

I think it would be good to have * automatically when allow is empty or undefined.

Yes, I agree.

And I can also suggest a middleware called request-limit based on this.

I think that renaming is not needed. And what do you think about allowing to receive (conninfo: ConnInfo) => boolean as IPLimitRule? It's smarter than your suggestion in PR for me.

Also, your PR includes 2 suggestions about allowing to add * by automatic and adding denyHandler and validHandler. Could you separation that?

[EdamAme-x]

thanks for reply
I understood. Wait a moment...

[EdamAme-x]

I think that renaming is not needed. And what do you think about allowing to receive (conninfo: ConnInfo) => boolean as IPLimitRule? It's smarter than your suggestion in PR for me.

nice suggestion! I like this.

How about this?
(remote: NetAddrInfo) => boolean

And, I think text of the denied error should be optional. What do you think? (Forbidden)

[EdamAme-x]

RFC (translated)
https://tex2e.github.io/rfc-translater/html/rfc9110.html#15-5-4--403-Forbidden

[EdamAme-x]

@nakasyou I just ready to review
nakasyou#3

[usualoma]

Hi @nakasyou
Great middleware!

Wildcard

Is “Wildcard” an important feature of this middleware? This is because, although the use of wildcards is seen as customary, I do not believe that such a notation is defined as an official specification.

Patterns with trailing *, such as 192.168.2.*, we can specify in the CIDR.
The pattern 192.*.2.* is, um, probably not used in real-world operations.

If you have a special preference for “Wildcard," you may support It, but otherwise, I think that not supporting it will reduce the amount of code and make future maintenance easier.

[EdamAme-x]

@usualoma
In my opinion, it needs to be a feature, just like Cloudflare supports wildcards.
(Some providers seem to offer the same range of IPs.)
However, I would prefer moving the function to another file. (e.x., “. /utils.ts”)

[usualoma]

@EdamAme-x
Thanks!
Sorry for my lack of knowledge, can you tell me in what scenarios Cloudflare allows you to use * for IP addresses? (I can't seem to find the information)

[EdamAme-x]

@usualoma
sorry, I guess I didn't check enough,
wildcard is already obsolete and CIDR seems to be in use.

[ryuapp]

Hi @nakasyou. Nice PR :)

How about reconsidering your naming conventions?
IMO, it is necessary to improve naming convention, since it is APIs that is exposed externally as part of utils.
e.g.)

ip-limit // this helper actually looks like an ip access manager.
distinctionRemoteAddr // "distinction" is a noun.
expandIPv6, ipV6ToBinary // variation in spelling(IPv6, ipV6).

I have a few other opinions, I'll leave them later.

[fzn0x]

How about reconsidering your naming conventions?

Agreed on this, I think we should go with ip-rate-limit, because it offers similar functionality.

[nakasyou]

@usualoma, thank you for comment, I removed wildcard as you said.

[codecov]

Codecov Report

Attention: Patch coverage is 96.90722% with 9 lines in your changes missing coverage. Please review.

Project coverage is 96.15%. Comparing base (fa5b742) to head (28ce298).

Files	Patch %	Lines
src/middleware/ip-restriction/index.ts	94.94%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #2813      +/-   ##
==========================================
+ Coverage   96.14%   96.15%   +0.01%     
==========================================
  Files         142      144       +2     
  Lines       14457    14748     +291     
  Branches     2622     2552      -70     
==========================================
+ Hits        13899    14181     +282     
- Misses        558      567       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nakasyou]

Hi @ryuapp, thank you for review.
I fixed some spelling errors from your comment.

Also I'd like to discuss about name of the middleware.
@fzn0x should name the middleware rate-limit, but I think the middleware doesn't do rate-limit. I think rate-limit is things that limit the number of accesses, what do you think?

[fzn0x]

Hi @ryuapp, thank you for review. I fixed some spelling errors from your comment.

Also I'd like to discuss about name of the middleware. @fzn0x should name the middleware rate-limit, but I think the middleware doesn't do rate-limit. I think rate-limit is things that limit the number of accesses, what do you think?

I don't have other suggestions other than ip-rate-limit, maybe you have an idea for other names.

[johnforte]

I don't have other suggestions other than ip-rate-limit, maybe you have an idea for other names.

Why not just ip-deny-middleware? Those make more sense than an ip-rate-limit.

[nakasyou]

Hi @johnforte, thank you for proposing.

Why not just ip-deny-middleware? Those make more sense than an ip-rate-limit.

It maybe suitable. However naming it middleware is not smart, so I think we should name it ip-deny if we name it ip-deny-middleware

[usualoma]

Hi @nakasyou

I think ip-limit is fine and a good enough option, although some of the others' suggestions get a nod.

Another option

By the way, I was thinking, what about turning this middleware into a middleware for "access control" that is not limited to IP addresses?

Assuming that the first level of support is restriction by IP address, it also supports authorization by authorization headers and cookies. I envisage something that would also be renamed to "access-control" and could be used as follows.

It could be used in production environment to "allow access if either the IP address or another authentication method matches".

import { Hono } from 'hono'
import { basic as acBasic, cookie as acCookie, accessControl } from 'hono/access-control'
import { getConnInfo } from 'hono/...'

const app = new Hono()

app.use(
  '*',
  accessControl(getConnInfo, {
    allow: [                         // allow if satisfy any of the following
      '192.168.0.2',                 // allow by IP address
      acCookie('auth', secretValue), // allow by cookie
      acBasic('user', 'password'),   // allow by basic auth
    ],
  })
)
app.get('/', (c) => c.text('Hello world!'))

Although the functionality overlaps with existing "basic-auth" and "bearer-auth", it may be possible to co-operate reasonably while sharing code.

If this option is adopted, control by cookies and request headers can be considered in a separate PR. In this PR, only the name should be changed and remain "middleware that restricts by IP address."

[MathurAditya724]

Amazing middleware! A small suggestion I would like to add, can we have a custom error handler function in the options?

[nakasyou]

@MathurAditya724, thank you. Yes, the middleware has.

app.use('*', ipLimit(getConnInfo, {
  deny: [],
  allow: ['127.0.0.1', '::1']
}, c => {
  // error handler
  return c.text('Invalid IP address', 403)
}))

[yusukebe]

Hi @nakasyou !

Sorry for my late reply. This is awesome middleware! I've left some comments. Check them!

[yusukebe]

@nakasyou

Great! It looks good to me. The CI is falling, but we can remove the error. Then, I'll merge this into the next later.

[yusukebe]

The time has come! Merging into the next. Thank you all!