Skip to content

fix(formatHost): support basic auth in host URL #219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

somprasongd
Copy link
Contributor

🔧 Description

This pull request adds support for Basic Authentication in the formatHost function by including username and password from the input URL into the formatted result.

📌 Motivation

Ollama does not provide built-in security or authentication mechanisms. To protect access to the service—especially in shared or production environments—many users set up a reverse proxy like Nginx to enforce Basic Authentication.

For example:

http {
    server {
        listen 80;

        auth_basic "Restricted";
        auth_basic_user_file /etc/nginx/.htpasswd;

        location / {
            proxy_pass http://localhost:11434;
        }
    }
}

With this configuration, requests to Ollama require valid credentials. In order for clients to successfully communicate through the reverse proxy, they must include credentials in the request URL (e.g., http://user:pass@host:port). This PR ensures that such credentials are preserved in the formatted host string, enabling compatibility with secured proxy setups.

✅ Changes

Preserve username and password (if provided) in the output of formatHost.

No impact on behavior for URLs without basic auth.

Maintains removal of trailing slashes and default port logic.

🧪 Example

formatHost('http://admin:1234@localhost:11434/')
// Output: 'http://admin:1234@localhost:11434'

🧪 Added Tests

Comprehensive test cases have been added for:

  • Username only
  • Username + password
  • Default port cases
  • HTTPS with credentials
  • Custom ports
  • URLs with and without trailing slashes

@josx
Copy link

josx commented Apr 13, 2025

I also need this support too.
Because i want to use cline which depends on this lib.

Copy link
Collaborator

@BruceMacD BruceMacD left a comment

Choose a reason for hiding this comment

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

This looks good, thanks for the contribution!

@BruceMacD BruceMacD merged commit 6a4bfe3 into ollama:main Apr 14, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants