feat(middleware): Introduce IP Limit 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

• [x] Add tests
• [x] Run tests
• [ ] bun denoify to generate files for Deno
• [x] 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. https://github.com/nakasyou/hono/pull/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 https://github.com/nakasyou/hono/pull/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[bot]]

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 :warning:

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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: 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."

[EdamAme-x]

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?

I thought the same thing. But in my opinion, restrictions in cookies and headers should be left to other middleware. Hono-users may be confused as to which middleware to use.

[EdamAme-x]

I would like to be able to pass IP address rules in this format as well. (for DataBase, RateLimit) (remote: NetAddrInfo) => boolean.

[nakasyou]

Hi @usualoma, thank you for proposing, but I don't agree. I have 2 reasons.

First, as @EdamAme-x said,

But in my opinion, restrictions in cookies and headers should be left to other middleware. Hono-users may be confused as to which middleware to use.

I agree that. It is reasonable to separate the IP address from other authentication methods for me. Currently we can limit by cookies and basic auth using middleware.

Second, users maybe not using IP address control, users may use only acCookie or acBasic. In this case, getConnInfo is not needed. Also it doesn't when code runs on runtime not supported getConnInfo such as Node.js or Vercel.

So I don't agree about your proposing.

A another suggestion, what do you think?

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

const app = new Hono()

app.use(
  '*',
  accessControl({
    allow: [                         // allow if satisfy any of the following
      controlIPAddress(getConnInfo, ['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!'))

[fzn0x]

controlIPAddress(getConnInfo, ['192.168.0.2']),    // allow by IP address

Hi @nakasyou it sounds like you can control the IP address.

Actually I have a new idea for the name!

It is....

ip-restrict 🎉

Because as I remember restriction is the most common naming when it comes to limit access to a network by IP.

[fzn0x]

ip-limit sounds like the IP have some limits.

other than ip-restrict we can try limit-ip-access or limit-ip-connect

[usualoma]

@EdamAme-x @nakasyou Thanks for the explanation!

Indeed, my proposal was mixing too many things.

I was hoping to have a control similar to Apache's Satisfy Directive. As it is currently not possible to get Satisfy Any behaviour just by combining the middleware provided in hono. https://httpd.apache.org/docs/2.4/en/mod/mod_access_compat.html#Satisfy

I calmed down and thought about it.

As for Satisfy Any, I have come to the conclusion that it is better not to mix it with this PR. I think This PR should remain as it is now, simple IP address restriction only. (Sorry for the confusion.)

It would be nice if Satisfy Any could be provided in a separate, independent middleware and combined with other middleware. I envisage the following APIs.

import { Hono } from 'hono'
import { ipLimit } from 'hono/ip-limit'
import { basicAuth } from 'hono/basic-auth'
import { satisfyAny } from 'hono/access-control' // New middleware
import { getConnInfo } from 'hono/...'

const app = new Hono()

app.use(
  '*',
  satisfyAny([
    ipLimit(getConnInfo, { allow: [ '192.168.0.2' ] },
    basicAuth({ username: 'hono', password: 'acoolproject' })
  })
)
app.get('/', (c) => c.text('Hello world!'))

import { Hono } from 'hono'
import { ipLimit } from 'hono/ip-limit'
import { bearerAuth } from 'hono/bearer-auth'
import { rateLimit } from '@app/my-rate-limit'
import { satisfyAny, satisfyAll } from 'hono/access-control' // New middleware
import { getConnInfo } from 'hono/...'

const app = new Hono()

// Very complex access restriction rules.
app.use(
  '*',
  satisfyAny([
    satisfyAll([
      ipLimit(getConnInfo, { allow: [ '192.168.0.2' ] },
      bearerAuth({ token })
    ]), // If both are satisfied, rateLimit is not necessary.
    rateLimit()
  })
)
app.get('/', (c) => c.text('Hello world!'))

[ryuapp]

I'm sorry for being late, allow me to leave comments.

Property name

Sorry for pointing out names again and again, but allowList/denyList expresses the nature better than allow/deny.

Custom Error

There is no need to implement it with this pull request, but it would be more convenient to be able to set a custom error.

Middleware name

limit means a restriction on the size or amount of something permissible or possible. IMO, restriction is better than limit. The problem is that the name is long.

Dependency on another middleware

It is undesirable to depend on getConnInfo(). This middleware exists only for IP restriction, so I don't think there is any need to depend on other middleware. I think users can use this middleware in any way they want by needing only for an IP.

import { Hono } from 'hono'
import { ipRestriction } from 'hono/ip-restriction'

const app = new Hono()

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

[nakasyou]

@usualoma

It would be nice if Satisfy Any could be provided in a separate, independent middleware and combined with other middleware. I envisage the following APIs.

Yes, I agree for it.

@ryuapp

Property name

It looks good for me, but it is longer, so discussing is needed.

Custom Error

It's good if we can do that.

Middleware name

limit means a restriction on the size or amount of something permissible or possible. IMO, restriction is better than limit. The problem is that the name is long.

I had thought that ip-limit is good, but I read your comment, I agree for your opinion. @fzn0x, could you tell me what you think?

Dependency on another middleware

Hmm, I can't understand. Could you explain in more detail?

[ryuapp]

@nakasyou

Dependency on another middleware

Hmm, I can't understand. Could you explain in more detail?

I thought about it again, I'm not really worried about dependencies, but rather the use of getConnInfo(). getConInfo() only works on certain runtimes. Therefore, this middleware only works with certain runtimes(Bun, Deno, Cloudflare workers). In order to support many runtimes, wouldn't it be better to only require an IP address?

[nakasyou]

Hi @ryuapp

In order to support many runtimes, wouldn't it be better to only require an IP address?

I don't understand your thinking well. Could you show me some examples about that?

And I have some questions for your example in https://github.com/honojs/hono/pull/2813#issuecomment-2156283139:

import { Hono } from 'hono'
import { ipRestriction } from 'hono/ip-restriction'

const app = new Hono()

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

• How does ipRestriction get IP address of request?
• What does 1st argument of ipRestriction mean?

[ryuapp]

Oops, sorry. I misundestood that. This middleware requires a function to get an IP address, doesn't it? Personally, I don't like depending on another helper, so you don't need to worry about this topic. It's OK.